Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove -mavenOptions argument #445

Merged
merged 1 commit into from
Feb 7, 2023
Merged

Conversation

basil
Copy link
Member

@basil basil commented Feb 7, 2023

I could not find anything using this argument in open-source or proprietary code, and its present just increases the complexity of the code for no reason. If a legitimate use case is found for this argument it can always be restored, but for now let us err in favor of removing unused code.

@basil basil added the removed label Feb 7, 2023
@basil basil requested a review from jtnord February 7, 2023 01:44
@basil basil requested a review from a team as a code owner February 7, 2023 01:44
@jglick
Copy link
Member

jglick commented Feb 7, 2023

I could not find anything using this argument

FTR because it was used temporarily when testing prerelease code, which was otherwise more difficult: #378 jenkinsci/bom#1372 (comment)

@jtnord
Copy link
Member

jtnord commented Feb 7, 2023

I could not find anything using this argument

FTR because it was used temporarily when testing prerelease code, which was otherwise more difficult: #378 jenkinsci/bom#1372 (comment)

possibly obsoleted by maven 3.9+ introduction of MAVEN_ARGS (ref)? (or by MAVEN_OPTS which is readily available today?)

@jglick
Copy link
Member

jglick commented Feb 7, 2023

possibly obsoleted by maven 3.9+ introduction of MAVEN_ARGS?

Yes going forward MAVEN_ARGS would be a good replacement. The same could be said for -mavenProperties and -mavenPropertiesFile (both of which are currently used by bom/pct.sh for example) since these are simply a special case of passing --define.

or by MAVEN_OPTS which is readily available today?

No because that sets JVM options, not Maven command arguments. As far as I know, without -mavenOptions there is no straightforward way to inject arbitrary arguments such as --activate-profiles to PCT when using Maven 3.8.x. Of course you could create a special wrapper (like the pipeline-maven plugin does) should the need arise.

Anyway, I was just noting the history for the benefit of any curious contemporary or future viewers.

@basil
Copy link
Member Author

basil commented Feb 7, 2023

Keeping this but renaming it to -mavenArgs because it aligns more closely with MAVEN_ARGS than with MAVEN_OPTS.

@basil basil merged commit 1e6d22d into jenkinsci:master Feb 7, 2023
@basil basil deleted the mavenOptions branch February 7, 2023 16:03
@basil basil added internal and removed removed labels Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants