some improvements... optional 😉 #55
Conversation
There was a problem hiding this comment.
Hi and thank you for your suggestions. At the moment I only have a few notes and one change request, but more may follow:
not everyone has Maven installed on the local machine
@Estrayl and I haven't decided whether to add the Maven wrapper yet. Personally, I think it's unnecessary, since all the major Java IDEs ship with Maven and I see no reason to support more exotic development environments.
that makes it possible to conquer all builds simultaneously
Do you mean the servers will build concurrently? Because the parent pom.xml already allows to build everything in one go as it is. Although this is not a feature we use. During development, the IDE takes care of tracking and updating changed classes, and for deployment we do individual builds, as you rarely need to update more than one server.
I'll test and see what practical benefits your changes bring when I find the time, as I haven't really done much with Maven modules yet.
I also have yet to test nightly.yml. Hopefully it has some kind of notification mechanism. If the reports have to be opened manually it is something that can go unnoticed for quite a while. We already have CodeQL, which notifies about vulnerabilities in dependencies, but it doesn't seem to be smart enough to detect them from transitive dependencies.
more to come. if you wish so 😉
Thanks again. We're always happy when someone decides to contribute instead of keeping improvements and fixes to themselves (which is very common in the Aion community 😄).
If you are unsure about future contributions, you can always discuss them beforehand on our Discord.
chat-server/pom.xml
Outdated
| <compilerArgs> | ||
| <arg>-Xlint:all,-preview,-this-escape</arg> | ||
| </compilerArgs> | ||
| <showWarnings>true</showWarnings> | ||
| <debug>true</debug> |
There was a problem hiding this comment.
Not sure if removing these parameters in all projects is a good or bad thing. Some of the warnings have been helpful in the past but I can't remember new valid ones in more recent years that I had to fix 🤔
There was a problem hiding this comment.
funny enough I couldn't build a single module with these parameters. I'll double-check.
|
thanks for the remarks @neon-dev. I've regarding the nightly job; no, there is no notification in place. an alternative for that would be adding an OSSHR upload & report action. those exist. I preferred the manual check up until now. edit: oh, sorry. you spoke about IDE's, not distributions. lol |
ok, I do see that, "one command to rule them all" is not really an addon. but, using the individual builds are still possible. let me know what you think of it, once you tried it. I've now created my "personal cherries" branch, so the PR is more focused on single changes. |
.github/workflows/nightly.yml
Outdated
| distribution: 'temurin' | ||
| cache: maven | ||
| - name: Dependency Report | ||
| run: mvn --no-transfer-progress versions:display-dependency-updates |
There was a problem hiding this comment.
removed the Maven wrapper usage, because it is no longer part of this PR.
bbortt
force-pushed
the
4.8
branch
2 times, most recently
from
b3a91b5 to
7f7f63e
Compare
|
this should do |
includes a `nightly.yml` build that reports available dependency updates. resolves [`CVE-2024-7254`](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-7254) being present in `com.mysql:mysql-connector-j:8.3.0`.
|
Hi, may I ask if this can solve the issue of inconsistent versions of "slf4j - API" in dependencies? |
Well, there is now one less conflict (one instead of two): It's not really a problem, the newer version was and still is selected for all the builds. You could define an explicit dependency on it, but with slf4j-api in particular, it's not something to worry about unless there's a vulnerability in the imported version. |
Yes, I saw the PR regarding this issue and would like to ask by the way. |
hi there! huge thank you for open sourcing all your code. I've recently installed my own server on a raspberry pi for private usage. it works like a charm 💌
being a developer myself, I've some remarks on the project. mostly convenience things. I leave it up to you if you merge it all, cherry-pick single commits or don't care at all. this PR just is my way to say "thank you".
./mvnw package. if you still want to build single packages, execute./mvnw package -pl :your-package(as beforehand). note that this might need adaptions to thedeploy.ymljob, I am not entirely sure.CVE-2024-7254being present incom.mysql:mysql-connector-j:8.3.0. I preferrably don't want vulnerabilities in production code.more to come. if you wish so 😉