Re: Any concrete plans after the GDB BoF?

[Andrew Burgess <aburgess@redhat.com>]

Joel Brobecker <brobecker@adacore.com> writes:

>> > Now, where this might save time is if we had some kind of git hook which
>> > could validate the code was formatted correctly and reject push attempts
>> > if they are not formatted.  Then I could stop thinking about
>> > formatting.  But until then .... I don't think reviewers will be able to
>> > stop asking: is this formatted correctly?
>> > 
>> 
>> That's what I have in mind. Some pre-commit hook that checks/does
>> things. Obviously we're not there yet, but that would be the most
>> convenient way.
>> 
>> I think anything that needs to be checked by hand wouldn't be an
>> improvement compared to the current process.
>
> As long as the the tool is able to do the formatting of a given file
> using only that one file, the git-hooks are ready. There is a
> style_checker option which is currently calling a script that does
> nothing. But if we have some tools that check things such as formatting,
> copyright header format, etc, it's easy to insert them and reject
> the commit.
>
> The problem is that this arrives too late in the process, IMO, because
> by then, the review has already gone through. For a formatting issue,
> one could argue that the change is trivial, and therefore automatically
> re-approved, but this is not ideal.

Whether the commit should be reformatted and auto-approved is orthogonal
I think to whether we should have an auto-format checked as part of the
commit hook.

As long as folk are able to manually push to master then the process is
open to (honest) user mistakes, and we should, as far as possible aim to
have systems in place to guard against those mistakes.

So having git refuse to accept a commit that is incorrectly formatted
would be a good thing; though I 100% agree with you that ideally we
would ALSO have tools that could auto-check the formatting earlier in
the process and bring that to the developers attention.

>
> What we would want is some kind of CI system which, from the moment
> the developer proposes a patch, gets the change validated through
> the CI. Style-checking is usually quick, so a good candidate for
> a CI. But how to integrate that without starting to re-invent
> one of those existing Git review or Git hosting systems that already
> exist?
>
> What this discussing is showing, IMO, is that the email-based system
> of proposing and reviewing patches may be fast and comfortable, but
> has some limitations compared to other more modern systems that seem
> hard to overcome. We can find technical solutions that overcome those
> issues, but each time I try this exercise, I find myself thinking
> the problem has already been solved is there for the taking if only
> we could accept email-based workflows might be degraded, possibly
> to the point where it becomes more productive to use the recommended
> interface (e.g. website).

100% agree.

>
> Good or bad, my concern is that the younger generation views emails
> as antiquated and at the same time they have grown up learning about
> collaboration using systems such as GitLab or GitHub.

I'd avoid the word 'antiquated' as it (too me) seems to have negative
connotations.

But I agree, many developers are familiar with a pull-request
development model, and I think it has many advantages over our current
way of doing things, I'd be very much in favour of switching to a PR
style system.

That doesn't mean there aren't also advantages to how we do things
today.

Thanks,
Andrew