A possible data point… For a while I worked on isomorphic-git (javascript) which had a pre-commit hook to format the code. It was rather surprising at first, but I ended up really liking it, more than projects that attempted to check the formatting during the build, which were susceptible to ways of running the build that skipped the format check.
David Jencks > On Oct 13, 2022, at 8:31 AM, Guillaume Nodet <gno...@apache.org> wrote: > > Le jeu. 13 oct. 2022 à 17:13, Łukasz Dywicki <l...@code-house.org > <mailto:l...@code-house.org>> a écrit : > >> Some of findings I have: >> >> - Static code analysis might be slow, reformatting code will add >> additional time to the build which might not be desired for developer >> experience >> > > This is usually reduced to 0 because the plugins just check on files that > have been modified since the last build. > > >> - Error messages produced after reformatting might mistake first time >> contributors who are not familiar with reformatting step done before >> compilation >> > > Not sure to understand what you mean. Formatting is not supposed to throw > errors... > > >> - I recently found a plugin for IJ: >> https://github.com/krasa/EclipseCodeFormatter >> <https://github.com/krasa/EclipseCodeFormatter> (allows to unify two >> dominant IDEs) >> > > Forcing specific a environment in IDE is not ideal imho. If you work on > different projects, you then need to activate / deactivate plugins or > your code style everytime you switch. > In addition, not enforcing the code style strictly leads to source files > being not inline with the formatter settings. WHich in turns means that > you can't really use it because whenever you do, you end up with many > changes unrelated to your current work that you need to undo in order to > keep your PR clean and minimal. > I think that's a real problem, and that would also be solved by an > automatic format imho. > > >> - Verification of code style might be automated via pull requests, if >> PRs are enforced. Otherwise core developers must take care of their >> habits or commit hooks. >> > > I still don't see the problems with an automatic format step. I've been > using it in projects for years in camel. The only problem is when people > don't build before committing, but that can't really happen in the maven > land > where almost all changes are done through PRs and validated. We just need > to enforce the no-file modified step in PRs. Ideally, we'd even have a > commit hook to enforce that. Not sure if that's doable. > > If there a real problems, what could be done is move the autoformatting > step in a separate profile which could be deactivated by default (though > people that want it can activate it in their settings). The PR workflow > would enable this profile and ensure there's no modification to the input > files. I really don't see the benefit of not running it for everyone, but > if some people do, that's fine with me. > > >> >> Cheers, >> Łukasz >> >> On 13.10.2022 15:27, Guillaume Nodet wrote: >>> Le jeu. 13 oct. 2022 à 12:50, Olivier Lamy <ol...@apache.org> a écrit : >>> >>>> On Thu, 13 Oct 2022 at 17:47, Tamás Cservenák <ta...@cservenak.net> >> wrote: >>>>> >>>>> Howdy, >>>>> >>>>> to not get lost in implementation details, it is really unimportant >>>>> (spotless, this or that...) >>>>> >>>>> What IS important is that we agree to implement "IDE agnostic source >>>>> formatting", that happens during build >>>>> (by maven plugin). This means _everyone_ will end up with 100% the same >>>>> result. >>>>> >>>>> The only thing that comes to my mind is "sloppy committer" who does not >>>>> build but pushes.... >>>> >>>> not sure to understand. You mean formatting sources will be bind to a >>>> phase and be executed automatically? >>>> >>> >>> Yes, I meant "automatic" in opposition to "on demand". Sorry if that was >>> not clear. >>> >>> >>>> >>>> Wouldn’t it be better to still have a check as today (checkstyle:check >>>> or the used foo-bar-formatter-plugin:check) which checks the sources >>>> at validate phase. >>>> Then in case of failure, the committer can just run >>>> foo-bar-formatted-plugin:format. >>>> So committer can apply some manual changes as well. >>>> this will detect "sloppy" committer as today by simply failing without >>>> adding another plugin running some scm command during the build. >>>> >>> >>> Well, the point is to avoid the multiple round trips between IDE / shell >> to >>> fix all validation issues. Also, you mentioned earlier that IDEs always >>> have some discrepancies in their formatters, and that's exactly the >> reason >>> why having a single reference formatter (i.e. that one that is run during >>> the build) is a good idea imho. >>> >>> >>>> >>>>> Is there some option to have a profile (activated on CI) that detects >>>> there >>>>> are "uncommitted changes" post build? >>>>> As that would mean that sourcetree contains sources NOT formatted, and >>>> that >>>>> CI build did reformat them... >>>> >>> >>> I'd like such a check for PRs. This would definitely ensure authors have >>> run the build before committing, >>> and most importantly, that the files are correctly formatted. >>> >>> >>>>> >>>>> But even without this step mitigation is simple: someone just rebuilds >>>> HEAD >>>>> and commits... >>>>> >>>>> Thanks >>>>> T >>>>> >>>>> On Thu, Oct 13, 2022 at 8:41 AM Tamás Cservenák <ta...@cservenak.net> >>>> wrote: >>>>> >>>>>> Formatting happens during build, so once merged all we need is to >> merge >>>>>> master into PR branches... >>>>>> >>>>>> T >>>>>> >>>>>> On Thu, Oct 13, 2022 at 8:40 AM Sylwester Lachiewicz < >>>>>> slachiew...@gmail.com> wrote: >>>>>> >>>>>>> What about already open PR with some features or bug fixes. >>>>>>> Don't we should review and merge it before reformatting code - >>>> otherwise >>>>>>> we >>>>>>> will have lots of merge conflicts and over time no one will have >>>> energy to >>>>>>> review it? >>>>>>> >>>>>>> Sylwester >>>>>>> >>>>>>> śr., 12 paź 2022, 18:23 użytkownik Guillaume Nodet < >> gno...@apache.org >>>>> >>>>>>> napisał: >>>>>>> >>>>>>>> I'd like to propose merging the following PRs: >>>>>>>> * https://github.com/apache/maven-shared-resources/pull/1 >>>>>>>> * https://github.com/apache/maven-site/pull/329 >>>>>>>> * https://github.com/apache/maven/pull/824 >>>>>>>> * https://github.com/apache/maven-resolver/pull/147 >>>>>>>> ... and more to come >>>>>>>> >>>>>>>> The idea is to use plugins to automatically reformat the source >>>> code and >>>>>>>> sort imports to obey the maven coding style. >>>>>>>> The first PR adds the necessary resources to maven-shared-resources >>>> : a >>>>>>> new >>>>>>>> header file, as there's a requirement to put the header at the very >>>>>>>> beginning for the import sorter plugin to work correctly (else it >>>>>>> considers >>>>>>>> the license comment to be part of comment headers and screws the >>>>>>>> formatting), and the eclipse xml formatter plugin. >>>>>>>> The second PR updates the web site to point to that file which >>>> would be >>>>>>> in >>>>>>>> git instead of on the maven web site, and also updates the >>>> instruction >>>>>>> for >>>>>>>> IDEA since it has been supporting the eclipse xml config for years >>>> now). >>>>>>>> The third and fourth PRs are updates on maven and maven-resolver to >>>>>>> apply >>>>>>>> those two plugins. >>>>>>>> >>>>>>>> Those plugins have been used on mvnd and maven-build-cache-extension >>>>>>>> already, although they are not using a single shared resource that >>>>>>> would be >>>>>>>> added by the first PR. Hence mvnd is still using its previous >>>> coding >>>>>>>> style. >>>>>>>> >>>>>>>> Another point is that those plugins are fast and only do not process >>>>>>> files >>>>>>>> if they have been already processed and untouched since the last >>>>>>> build. So >>>>>>>> from a daily development pov, this is transparent and does not >>>> incur any >>>>>>>> additional processing time during the build (or not much really). >>>>>>>> >>>>>>>> Cheers, >>>>>>>> Guillaume >>>>>>>> >>>>>>> >>>>>> >>>> >>>> --------------------------------------------------------------------- >>>> To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org >>>> For additional commands, e-mail: dev-h...@maven.apache.org >>>> >>>> >>> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org >> For additional commands, e-mail: dev-h...@maven.apache.org >> >> > > -- > ------------------------ > Guillaume Nodet
