On Wednesday, February 24, 2021 at 4:32:23 AM UTC-7 James wrote:
> Hi all, > > So I have some strong opinions here and these are that nothing should ever > be auto applied. > > Whatever format you have defined it will produce worse code in some > situations, and this results in code littered with > //AUTO_FORMAT_IGNORE:next_10_lines which does not aid to the readability of > the code or code that is just harder to read than it should be. > > I have found that what works best is if there is a way for users to format > my changes "on demand" by say running a specific maven goal, or having a > specific IDE configuration (not all IDEs can format the same way so this > last one is hard) , *and *for the CI to flag deviations as part of the > code review. ie it should not fail the build, but say "this line violates > rule line_length > X". That way as a maintainer I can choose to accept > something that I believe is legible when the committer has left a comment > saying why the code deviates, or just leave it as the CI saying "please fix > your style issues". > > There is also nothing worse than running `mvn -Dmaven-surefire-debug=XYZ > test -Dtest-foobar` and having all the breakpoints messed up because the > source code changes underneath you, or running tests from your IDE, > committing and then pushing only for the CI to fail with something that is > probably not easy to see (exit code 1 without any CI check comment?) > > What I feel is also missing here is what issue is this attempting to > solve. It is proposing a solution - but what is the exact problem > maintainers are seeing; perhaps there are better ways to solve that? > > I was trying to avoid changes in indentation, braces, and comments that I felt did not add value to git plugin and git client plugin pull requests. I was spending time reminding submitters that the contributing guide in those plugins specifically asked that they not make white space changes without consulting the maintainers. I was occasionally frustrated when I failed to follow the contributing guidelines for that plugin in my reviews. Then white space changes would arrive in the code even though the guidelines said they should not. Worrying about formatting during code review seemed like a waste of time. Two years ago the git plugin had over 60 pull requests open proposing various changes. A white space or formatting change in one of those pull requests would cause another pull request to have a conflict. Since that time, I've resolved the issue in the git plugin another way. I declared pull request review bankruptcy and closed 50+ pull requests because I was clearly not making progress reviewing them. Since then, I've implemented mandatory code formatting in another plugin (platformlabeler) as an experiment and have found the code formatting helpful. The one open pull request on that repository has been complicated by a "diff wall" created by a change from two space indentation to four space indentation, but that diff wall was not difficult to handle in that case. I still remember with affection the experience watching a team adopt automated formatting of their Java code just because Kent Beck's Extreme Programming book said to do it. We were feeling bold and wanted to go "all in". It removed one area of friction and allowed them to work better together. I suspect the `go fmt` command is similarly used to reduce friction due to formatting styles. I've accepted that the git plugin and git client plugin will likely never use automated formatting because of the "diff wall" it would create. Others have described techniques to avoid or manage the "diff wall", but the formatting in those two plugins is such a mess that I believe the "diff wall" is unavoidable in their cases. Mark Waite -- You received this message because you are subscribed to the Google Groups "Jenkins Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/8028b729-05a8-43d2-bd9f-05460b188f94n%40googlegroups.com.
