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

fix methods of lint and warning see https://github.com/codehaus-plexus/plexus-compiler/pull/131#discussion_r622796309 #132

Merged
merged 1 commit into from
May 2, 2021

Conversation

olamy
Copy link
Member

@olamy olamy commented Apr 29, 2021

No description provided.

@bondolo
Copy link
Contributor

bondolo commented Apr 29, 2021

The reason that the naming was a bit odd was that there was an existing showWarnings() flag. The intention was that the warnings flag would still control whether lint would be shown to avoid the weird corner case where warnings were disabled and lint was configured.
warnings enabled : show errors, warnings, and "lint"
warnings disabled : show errors only.

As proposed warnings and lint will be configured independently. This sort of makes sense for javac but will allow for the lint-but-not-warnings state. For Eclipse ecj, which has only one warnings command line flag but previously no method of giving it options, the configuration of warnings will be confusing. If you specify lint options but disable warnings then your lint options will be ignored.

For the <showLint/> usage which would enable -Xlint but provide no options I do agree that this should be possible. I am not sure if it is better to use distinguishing between null and empty string or a separate boolean flag.

@olamy
Copy link
Member Author

olamy commented Apr 30, 2021

@pzygielo ping

@pzygielo
Copy link
Contributor

👍

@olamy olamy merged commit bfd5875 into master May 2, 2021
@olamy olamy deleted the fix-lint-warnings branch May 2, 2021 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants