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

[MNG-7713] Make Maven fail if option present #1021

Merged

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Mar 1, 2023

As with previous PR (simple removal) the -llr got interpreted as -l lr, it logged all output to lr file. This would maean that use of -llr would still sneakily 'work' and probably cause surprise down the road to users.

Returned the option, and expicitly checking for it's presence to be able to fail with meaningful message.


https://issues.apache.org/jira/browse/MNG-7713

As with previous PR (simple removal) the `-llr` got
interpreted as `-l lr`, it logged all output to `lr`
file. This would maean that use of `-llr` would still
sneakily 'work' and probably cause surprise down the
road to users.

Returned the option, and expicitly checking for it's
presence to be able to fail with meaningful message.

---

https://issues.apache.org/jira/browse/MNG-7713
@cstamas cstamas self-assigned this Mar 1, 2023
@@ -280,6 +280,12 @@ public CLIManager() {
.desc("Ineffective, only kept for backward compatibility")
.build());

// Adding this back to make Maven fail if used
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add info about conflict with -l lr - why we need it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need it not only to avoid conflicting -l lr but also to emit sane error message (so we really want to parse it from CLI but fail if present). So, I started a new "section" here (below the "ineffective section") for those that should fail.

@cstamas cstamas merged commit 5a55231 into apache:maven-3.9.x Mar 1, 2023
@cstamas cstamas deleted the maven-3.9.x-MNG-7713-mvn-llr-fail branch March 1, 2023 12:30
cstamas added a commit to cstamas/maven that referenced this pull request Mar 1, 2023
As with previous PR (simple removal) the `-llr` got
interpreted as `-l lr`, it logged all output to `lr`
file. This would maean that use of `-llr` would still
sneakily 'work' and probably cause surprise down the
road to users.

Returned the option, and expicitly checking for it's
presence to be able to fail with meaningful message.

---

https://issues.apache.org/jira/browse/MNG-7713
cstamas added a commit that referenced this pull request Mar 1, 2023
As with previous PR (simple removal) the `-llr` got
interpreted as `-l lr`, it logged all output to `lr`
file. This would maean that use of `-llr` would still
sneakily 'work' and probably cause surprise down the
road to users.

Returned the option, and expicitly checking for it's
presence to be able to fail with meaningful message.

---

https://issues.apache.org/jira/browse/MNG-7713
@gnodet gnodet mentioned this pull request May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants