formatter-maven-plugin: New formatter config must invalidate cache
Describe the bug A clear and concise description of what the bug is.
Versions (OS, Maven, Java, and others, as appropriate):
- Affected version(s) of this project: 2.14.0
- OS: macOS Catalina 10.15.7
- Others:
To Reproduce
git clone git@github.com:walles/jedis.gitcd jedisgit checkout walles/format-on-mvn-validate-in-ci-2019mvn compile(this will run the formatter and initialize the cache)- Remove all the rules from
hbase-formatter.xml mvn compile- Note that nothing was reformatted, even though you just changed the rules.
rm target/formatter-maven-cache.propertiesmvn compile- Note that lots of files got reformatted now, after you removed the cache.
Expected behavior Files should be reformatted when the formatting rules change.
Actual behavior Nothing gets reformatted, even if the formatting rules change.
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Reactions: 1
- Comments: 24 (16 by maintainers)
Commits related to this issue
- Bump plugin and update cache javadoc * Bump formatter to latest release for current build * Update the javadoc for the cache to warn about changes a user might make that would invalidate any cache ... — committed to revelc/formatter-maven-plugin by ctubbsii 3 years ago
- Bump plugin and update cache javadoc (#455) * Bump formatter to latest release for current build * Update the javadoc for the cache to warn about changes a user might make that would invalidate a... — committed to revelc/formatter-maven-plugin by ctubbsii 3 years ago
In any case, all of this is philosophical. If somebody wants to do the work, then they can submit a PR to do it. If it’s not so substantial that it creates a maintenance burden, then I wouldn’t object to it. If nobody that thinks it is important wants to do the work, then the whole conversation is moot.
Coming from https://github.com/revelc/impsort-maven-plugin/issues/48.
IMHO, this is too black and white. What plugins, frameworks etc. should do is to help users as good as they can. If there are corner cases which are too complex to cover: So be it! A small documentation hint is usually enough (e.g. “This plugin takes A, B, C into consideration when calculating the hashes. You might need to wipe the cache manually in case you changed X,Y, Z.” or so).
This might be true for the user altering the config or updating the plugin. But once you pushed your change and it lands in the local workspaces of dozens of contributors/team members, it’s mostly out of your hand. Sure, leaving the cache in
targetwould solve this, but after more than a decade of Maven experience, I’ve seen most people runmvn clean ...all the time! I don’t say it’s good practise but it’s just the way the cookie crumbles. Take Quarkus for example: 900+ modules. After people have pulled, they ususally do amvn -Dquickly -Txwhich runs alsoclean. (If that profile didn’t includeclean, most people would type it on their own.) So you end up re-running the plugin on thousands of java source files for basically nothing.And at this point I have to ask: What happens more often? A cache-relevant config change or a
clean? Definitelyclean, AFAICS. So for many projects, leaving cache intargetis almost as “bad” as having no cache at all.Btw, Checkstyle is considering the config for calculating the hashes: https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/PropertyCacheFile.java I haven’t checked whether they add the config cache to each file entry or whether they have a separate key for that.
PS: Having said that, it’s good that there now is an explicit hint in the plugin doc that you might (will) have to wipe the cache on your own. When I moved the cache out of target in Quarkus in March this year, there wasn’t such a hint yet.