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

Add fix guidance to default check error message #533

Merged
merged 1 commit into from
Apr 2, 2023

Conversation

scordio
Copy link
Contributor

@scordio scordio commented Apr 1, 2023

Closes #531.

This changes the check mojo default error message from:

Some files do not have the expected license header

to:

Some files do not have the expected license header. Run license:format to update them.

Two questions:

  • Should I manually update the error message in the files under docs/reports/4.3-SNAPSHOT?
  • If I execute mvn clean verify, some non-Java files in scope for git are modified. Am I supposed to push these changes as well?

@hazendaz hazendaz self-assigned this Apr 2, 2023
@hazendaz
Copy link
Collaborator

hazendaz commented Apr 2, 2023

@scordio On your two questions.

On both, the docs are auto generated. I am not 100% on the first one needing something manual but @mathieucarbou can confirm. On the second issue, very aware of that issue myself, just discard when you see that for now. We do need to make that better. In the past I used to commit those separately then never push as you may have as well, then I typically go away and don't come back for weeks or months then get very confused why those exist. Its not a good contributor experience let alone new contributor experience. If you have time to look into how we might make that not occur, awesome, if not, think enough of an answer here to address.

On the change, wow we have that in a lot of places, Pressure sure we can use some sort of static to write once rather than many, but keeping scope of the change as-is. Thank you for both inquiring first to not waste your time and second for taking the time to contribute. It is very much appreciated and hope to see more contributions. The approach here I very much like as I noted on your original issue question regarding this. Its extremely helpful to tell a user how to address anything rather than just error. Recently even maven owned plugins are going a long way to make it more user friendly in that way. Clearly when someone failed to do what was needed such as run the plugin, they need a boost. There are other ways to automate such a condition such as a git hook but clearly getting all to install is yet another issue. So this along is worth its weight in gold IMO.

@hazendaz hazendaz merged commit e8f79d7 into mathieucarbou:master Apr 2, 2023
@hazendaz
Copy link
Collaborator

hazendaz commented Apr 2, 2023

One other note just as an FYI, we did just release 4.2. So we may be a bit before 4.3. That could change though depending how fast we can address some long standing issues we have with slow performance in general when using svn or git extensions to do timebased checks. Its also feasible that maybe we run a little faster than before but either way, it will probably be a few months if I had to guess before we release next cut so we have at least a good number of changes.

@scordio scordio deleted the gh-531 branch April 2, 2023 08:32
@hazendaz hazendaz added this to the 4.3 milestone Sep 25, 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.

Add suggestion to default errorMessage on how to fix missing/wrong license headers
2 participants