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

Report artifact info #75

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Report artifact info #75

wants to merge 3 commits into from

Conversation

jsoref
Copy link

@jsoref jsoref commented Mar 18, 2019

When an error occurs, this information is provided by handleArtifacts.

For warnings, the only information is the information provided by the getLog().warn() call itself.

Without this change, the following output is common:

[INFO] --- maven-enforcer-plugin:3.0.0-M2:enforce (display-info) @ acceptance-test-harness ---
[WARNING] Invalid bytecodeVersion for module-info.class: expected 52, but was 53
[WARNING] Invalid bytecodeVersion for module-info.class: expected 52, but was 53
[WARNING] Invalid bytecodeVersion for module-info.class: expected 52, but was 53
[WARNING] Invalid bytecodeVersion for module-info.class: expected 52, but was 53
[WARNING] Invalid bytecodeVersion for module-info.class: expected 52, but was 53
[WARNING] Invalid bytecodeVersion for module-info.class: expected 52, but was 53
[WARNING] Invalid bytecodeVersion for module-info.class: expected 52, but was 53

@batmat
Copy link
Member

batmat commented Mar 19, 2019

@jsoref can you confirm which version of extra-enforcer-rules you're using?

This should not be reported anymore since #36

@jsoref
Copy link
Author

jsoref commented Mar 19, 2019

@batmat: I have no idea how to answer your question.

That said, I claim that my patch is correct, because even if it doesn't happen today due to some hand waving the code is still incredibly unhelpful and should be fixed. It's also buggy in that in theory it could complain about the major version when the problem is only the minor version.

@mfriedenhagen
Copy link
Member

@jsoref, first of all, thanks for creating a PR.

  • As far as I understand your patch, all it does is to repeat the artifact's coordinate.
  • However, you normally know in which project you currently run Maven and
  • the [INFO] line above the warnings shows the artifactId so I do not see the immediate need for repetition of information unless you always run Maven in quiet mode, then groupId:artifactId could be handy.
  • You stated something about major/minor, that I do not understand. As far as I know the bytecode version only changes between major Java versions, i.e. stay the same between 1.8.0_132 and 1.8.0_242 resp. 11.0.2 and 11.0.6.

When an error occurs, this information is provided by handleArtifacts.

For warnings, the only information is the information provided by the getLog().warn() call itself.

Without this change, the following output is common:
[INFO] --- maven-enforcer-plugin:3.0.0-M2:enforce (display-info) @ acceptance-test-harness ---
[WARNING] Invalid bytecodeVersion for module-info.class: expected 52, but was 53
[WARNING] Invalid bytecodeVersion for module-info.class: expected 52, but was 53
[WARNING] Invalid bytecodeVersion for module-info.class: expected 52, but was 53
[WARNING] Invalid bytecodeVersion for module-info.class: expected 52, but was 53
[WARNING] Invalid bytecodeVersion for module-info.class: expected 52, but was 53
[WARNING] Invalid bytecodeVersion for module-info.class: expected 52, but was 53
[WARNING] Invalid bytecodeVersion for module-info.class: expected 52, but was 53
@jsoref
Copy link
Author

jsoref commented Feb 11, 2024

@mfriedenhagen:

  • I guess? All that the current code does is repeat an incomprehensible warning
  • I almost never know the thing that's running maven because I'm usually making PRs to random repositories and having random CIs call maven.
  • There's no reason for a random user reading a random log to realize that the info above is remotely related to the next line. That isn't how well behaved programs work.
  • I don't know that this is a promise. Maybe it is, maybe it isn't.

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.

4 participants