Re: automated formatting

[David Kastrup]

Dan Eble <address@hidden> writes:

> On Jan 28, 2020, at 12:11, Carl Sorensen <address@hidden> 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