Dan Eble <d...@faithful.be> writes: > On Jan 28, 2020, at 12:11, Carl Sorensen <c_soren...@byu.edu> wrote: >> Can you provide me with a presubmit hook that applies fixcc? Can you >> guarantee that, if fixcc has run on the code, there will be no further >> remarks on code formatting during review? >> >> I think that it's a really good idea to have presubmit hooks. I >> believe, but can't guarantee, that if all code were automatically >> reformatted on submission (by either fixcc or clang-format) there >> would be virtually no comments on formatting. And you could >> completely ignore any that were made. > > I'm all for making it possible for a contributor to set up something > like this if he pleases, but I think it's a bad idea to rely on this > level of automation and integration with a specific source-control > tool. > > I don't trust restyling programs to do the most desirable thing for > readability 100% of the time, and I especially don't trust them in > certain situations, such as when I'm checking in incomplete work > because of an urgent need to switch to another branch.
To clarify: before you joined the project, there were recurrent reformatting passes preferably at "quiescent" times to bring consistent style into the code base. There were large discussions about it but it was sort-of agreed that having a uniform style presented to people reading the code was a reasonably good idea and that the tool chosen and maintained did not cause noticeable damage (formatting of scm was a more tricky proposition and the respective script available by now just defers to Emacs for doing it). > Also, our basic goal is that none of the cooks poisons the soup, > i.e. merges badly formatted code to master. If some of them want to > experiment with wild mushrooms on the side, micromanaging their > commits doesn't guard the soup any better than checking when they > present their work for integration. > > If you build a style check into "make check", then it will happen > whenever contributors test their code and it will happen during the > review countdown. If you also build a style check into the final > tests before merging staging to master, the goal will be achieved. — The problem with precommit hooks is that you may end up formatting unrelated submissions. Doing it manually, you may end up with git add -p in order to just format your own work. Basically, no real great solution offered itself then (and I don't see that we can do this significantly better now, even while the particular tool to use may be subject to debate) and the adapted solution was to encourage using the fixcc scripts on one's own contribution and regularly apply it to the repository at reasonable points of time. I am not sure how far this policy dates back, but while Graham was project leader, it was done regularly. There was no intent by me not following this practice: I simply dropped the ball. I'd be willing to pick up on it since the adopted compromise was sort of agreed upon by the active developers. The set of active developers has changed by now, and of course we also try angling for the mystic developer just missing one particular tool/workflow in order to become productive or productive again. There is little enough sense in turning this into a showdown rather than an attempt of finding a consensus everyone can find themselves able to live with. I'll do (respectively already have done in a few issues) what I feel brings the comparatively simple fixcc tool back up to scratch (fittingly for my non-use of it, its --sloppy option was broken) and then hopefully we'll be in a better position of deciding how to continue. -- David Kastrup
