Thanks so much for bringing this to our attention Benoit. I agree that we should pave the contribution road and make it as least painless as possible.
I am in favor of removing the ratchet from Spotless and doing a bing bang. Since such a PR would practically not be reviewable, I guess ideally a committer or PMC member should do that. Once the commit lands, we can skim through the changes in IDEA to make sure Spotless did not plant a backdoor or something. One thing I am curious about is... *Do we all agree on the current Spotless configuration?* Styling is pretty subjective and we should ideally have one big bang. For instance, `log4j-tools` has a different Spotless configuration <https://github.com/apache/logging-log4j-tools/blob/master/log4j-tools-parent/pom.xml#L257> than Log4j and it doesn't use a particular Spotless Java formatter (e.g., Palantir's), though this was a deliberate decision to make that configuration align with the provided `.editorconfig` <https://github.com/apache/logging-log4j-tools/blob/master/.editorconfig>. Put another way, I would prefer a consistent Spotless+EditorConfig combo across all Log4j projects. Such that we can ship this in `logging-parent` and (hopefully) be done with formatting. On Tue, Mar 7, 2023 at 8:46 AM Benoit Lacelle <benoit.lace...@solven.eu> wrote: > Hello, > > Following Piotr P. Karwasz advice, I follow-up here a conversation from > https://github.com/apache/logging-log4j2/issues/1317 > > The main reason for my presence here is as author of Cleanthat ( > https://github.com/solven-eu/cleanthat) a Java linting engine. I'm a > contributor in Spotless, a multi-language formatting engine (in which > cleanthat has been integrated). I'm not a contributor in Log4J2. > > The issue in https://github.com/apache/logging-log4j2/issues/1317 is: some > Log4J2 contributors has issues related to Spotless formatting checks. In > this specific case, a user forked the repository, improved various files > (spelling issues), but it's CI job are failing due to Spotless. > > In fact, Spotless has been configured with a given set of rules, and the > ratchetFrom feature has been used not to require an initial > massive-reformatting commit. This leads to each file requiring to be > formatted by the first commiter (after Spotless integration). > https://github.com/apache/logging-log4j2/issues/1317 author issue is the > unexpected need to clean files around lines unrelated to his changes. > > Log4J2 team may decide to drop ratchetFrom feature, it would lead to a > massive refactoring. I would not advise doing so. > Log4J2 team may decide to keep ratchetFrom feature, but process all/some > files given all/some reformatting rules (e.g. re-process all > licenseHeaders). > Log4J2 team should probably clarify this situation (the need to reformat > the whole file as first committer since Spotless integrations) for > contributors around > https://github.com/apache/logging-log4j2/blob/2.x/CONTRIBUTING.md > > Cleanthat proposes a Github app to automatically fix PullRequests around > Spotless configuration. https://github.com/marketplace/cleanthat/ It would > help fixing such situations by cleaning automatically all files of a given > PR (especially around lines not relevant to the PR author (e.g. fixing the > license Header)). However, it would not cover > https://github.com/apache/logging-log4j2/issues/1317 (as it is actually an > issue in a fork of a fork of Log4J2 repository, and the PR has the fork as > base, while Cleanthat Github app would cover PR with Log4J2 as base > repository). > > FYI, I pushed this case to @nedtwig (Spotless author). We agreed the use of > ratchetFrom with HEAD~31 and `fetch-depth: 32` is *very clever*. > > -- > Benoit Lacelle > Java/ML/BigData Elite Engineer > +33 6 78 83 92 66 > benoit.lace...@solven.eu >
