search

PlayPro / CoreProtect

CoreProtect is a blazing fast data logging and anti-griefing tool for Minecraft servers.
Artistic License 2.0
660 stars 334 forks source link

Maven run-time utilizing Maven Wrapper #DevEx #602

Open dlehammer opened 3 months ago

dlehammer commented 3 months ago

The Maven version supported by this code-base isn't specified in the documentation. This is a potential risk, as the current implementation might not be compatible with new Maven releases.

I suggest encapsulating this in the code, by utilizing the Maven Wrapper a.k.a. mvnw. Hence no need to document or force the user to setup a environment for a specific Maven-version, instead this is handled implicitly 🤓

dlehammer commented 2 months ago

Hi @Intelli,

I can see you've added a "declined" label, but not elaborated on the reason.

If you know the Maven version used, I can take a stab at a PR 🤓

Intelli commented 2 months ago

CoreProtect always targets the latest Maven release

CyberFlameGO commented 2 months ago

I think this is also slightly less relevant for most developers looking to target CoreProtect's API - I say this because you've linked to the documentation, but the built API is published to a Maven repository, and therefore those building against the API can just fetch the dependency from the Maven repository instead of building the project themselves. Maven wrapper usage is funnily enough not very common - in Gradle-based projects it is a pretty standard practice but Maven hasn't historically faced the same significant breaking changes with their build tool. Regardless this was a cool suggestion to read through because I had not heard of the official one until now

dlehammer commented 2 months ago

Hi @Intelli,

I've created a Wrapper PR based on the current Maven (v3.9.8) release :)

dlehammer commented 2 months ago

Hi @CyberFlameGO,

Thanks for taking the time to reply, regarding:

I think this is also slightly less relevant for most developers looking to target CoreProtect's API - I say this because you've linked to the documentation, but the built API is published to a Maven repository, and therefore those building against the API can just fetch the dependency from the Maven repository instead of building the project themselves.

My aim is to minimize ambiguity by encapsulating (part of) the build tooling for this repo, my intention is to lower the bar for any contributor to this repo going forward. By codifying it with a wrapper, there's no need to also document how to configure a Maven development environment.

On top of that you can easily test out new Maven versions in a branch by changing the version in the .mvn/wrapper/maven-wrapper.properties.

Maven wrapper usage is funnily enough not very common - in Gradle-based projects it is a pretty standard practice but Maven hasn't historically faced the same significant breaking changes with their build tool.

Heh, I hadn't even spotted the build.gradle 🙈 I would similarly suggest the Gradle wrapper, for the same reasons 🤓

stale[bot] commented 1 week ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.