-
Notifications
You must be signed in to change notification settings - Fork 459
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
mvn spotless:apply
now prints out statistics
#1507
mvn spotless:apply
now prints out statistics
#1507
Conversation
I would categorize it like this: I think the PR as-is would be confusing because the first run would say that "100 files were considered" and after a few runs it would say "0 files were considered". The files were considered, a thorough up-to-date check showed that they were already clean :) |
Here is another step forward. I introduce a Formatter.name property that I'm not 100% comfortable with. The log now looks like:
I was keen to add the equivalent log in Gradle, but I'm too weak in Gradle. If this is a relevant improvment, I would appreciate a pointer to do so. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the change, it's close to mergeable.
Re: the Gradle plugin, it's tough because the up-to-date checking happens within the gradle machinery, and I'm not aware of a way to get a count of how many were skipped due to up-to-date checking. Regardless, they're not coupled, so it can always be added later.
plugin-maven/src/main/java/com/diffplug/spotless/maven/ImpactedFilesTracker.java
Outdated
Show resolved
Hide resolved
plugin-maven/src/main/java/com/diffplug/spotless/maven/ImpactedFilesTracker.java
Outdated
Show resolved
Hide resolved
plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Add an entry under ###Added
in plugin-maven/CHANGELOG
and I'll press merge.
mvn spotless:apply
now prints out statistics
README entry added. I also added a condition so that synthesis kicks-in only if |
I changed the Are these changes okay with you? If so I will merge. |
Thanks for the improvments. |
@nedtwigg I'm not convinced (anymore) about the WARN in case of not a single matching file. As the maven-plugin is executed per module, we may typically have a |
A fair amount of traffic on our bug tracker is
I'm happy to backtrack on this now or in the future, but it seems like it would be better to have a way for child pom to say e.g. |
@nedtwigg I'm also a user encountering many "it's not formatting any files". Which is one motivation for this PR. However, I foresee many WARNs in my own builds. I'd like to run a SNAPSHOT with one of my repository (to get a real feedback): do you know how I can deploy a SNAPSHOT in mvn local repository with the new setup ? (based on https://www.benediktritter.de/maven-plugin-development/) |
I think But that's a good point, I think we can use JITPACK again now too. I just turned JITPACK building back on, and it looks like it's working okay as of current master https://jitpack.io/com/github/diffplug/spotless/4c7b616e510bd83167ff329361ad819fac28ae6e/build.log Might be broken, not 100% sure... |
Ah, JITPACK is broken, working on that... |
fails with:
Same for
is OK. |
A run of
This is many warnings in my perspective. Regarding people with wrong includes: what's often lacking to me (with spotless) is proper loggings (e.g. this PR adds a DEBUG log when cleaning a file). I'm not sure (...) people with issues would try |
I am not familiar with Maven culture. Defining a bunch of goals in the parent pom to apply to children but don't have any effect seems like bad practice. But I'm not a maven user, and I'm happy to merge whatever native maven users feel the default behavior ought to be. |
In my perspective, this is totally legit and standard practise. But I'm just a single user. Do you know any other mvn Spotless user which may give his opinion on this? |
I always lean towards PR author's preference, merged :) |
Published in |
Follows #1506