llvm dev - May 2016 - [RFC] Helping release management

> On May 5, 2016, at 10:08 AM, Reid Kleckner via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> On Wed, May 4, 2016 at 5:07 PM, Quentin Colombet via llvm-dev <llvm-dev
at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> Typically, what I had in mind was things like typos/thinko, that are bugs,
that we notice a few minutes after we made the “main” commit. I do not want we
have to file a PR that is going to repeat what we are going to say in the commit
message.
> 
> Yeah, I agree, we shouldn't have to file PRs for that kind of stuff.
Quick fixes for the build or tests on other platforms obviously fall into this
category.
> 
> Do release managers have problems keeping track of these kinds of changes
in practice, though? You can always cut the branch from some quiescent period on
the weekend of night before the cut.
I’ve been doing quite a bit of that lately. The problem is not when you cut the
branch, but how you maintain it. For multiple months after you branched, you’re
willing to take fixes for miscompiles. So you have to got through every commit
and gauge the risk/benefit, see if it applies, if it passes the tests…

Anything that helps tracking that a particular commit need followup commits to
be fully functional would be a win. I think just mentioning the original commit
in the followups is a minimum (then you can have tooling to relate the commits),
but I don’t see a way to enforce this.

Ideally every commit message would be very explicit about what the commit is:
 - a compiler crash fix (with a description of the crash trigger)
 - a miscompile fix
 - an optimization to the compiler code
 - an optimization to the generated code
 - a refactoring
It’s not always easy to distinguish between the first four, and you really want
to understand it before cherry-picking in a long living branch.

Oh, and PRs are nice, but when the commit message is just “Fix PR1234”, it’s
still quite some work to figure out what to do with the commit :-)

Fred
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160510/f6965811/attachment.html>

On 11 May 2016 at 01:48, Frédéric Riss <llvm-dev at lists.llvm.org>
wrote:
> I’ve been doing quite a bit of that lately. The problem is not when you cut
> the branch, but how you maintain it. For multiple months after you
branched,
> you’re willing to take fixes for miscompiles. So you have to got through
> every commit and gauge the risk/benefit, see if it applies, if it passes
the
> tests…
Indeed!

However, during our "commit message" discussion [1], the consensus was
that too many rules means everyone will forget one or the other and
the result will be worse. So we have to be careful with what we ask.

> Anything that helps tracking that a particular commit need followup commits
> to be fully functional would be a win. I think just mentioning the original
> commit in the followups is a minimum (then you can have tooling to relate
> the commits), but I don’t see a way to enforce this.
The commit message guideline [2] already says that:

 * If the commit is a bug fix on top of another recently committed
patch, or a revert or reapply of a patch, include the svn revision
number of the prior related commit.

> It’s not always easy to distinguish between the first four, and you really
> want to understand it before cherry-picking in a long living branch.
Right now [2], we already have [TAG]s, PR mentions, and NFC flags to
account for most of what you want, but we do not have the set { opt /
codegen / crash } for what the fix is.

It is an interesting idea, and I try to do that myself as free text as
often as possible. But even if we add that, it still will be
considered "guideline", not a "rule", and therefore
candidate for
being ignored like all the others.

I personally like rules, and wouldn't mind some git hooks on commit
messages, but the risk of false positives and maintenance of hooks is
one that we cannot ignore. The alternatives are to either close down
the commit access like the GCC folks do, or to impose a culture of
backlash like the Kernel folks do.

That was discussed [1] and the consensus is that we prefer to have
guidelines and keep reminding people to be responsible.

cheers,
--renato

[1] http://lists.llvm.org/pipermail/llvm-dev/2014-September/077089.html
[2] http://llvm.org/docs/DeveloperPolicy.html#commit-messages

> On May 11, 2016, at 5:44 AM, Renato Golin <renato.golin at
linaro.org> wrote:
> 
> On 11 May 2016 at 01:48, Frédéric Riss <llvm-dev at lists.llvm.org>
wrote:
>> I’ve been doing quite a bit of that lately. The problem is not when you
cut
>> the branch, but how you maintain it. For multiple months after you
branched,
>> you’re willing to take fixes for miscompiles. So you have to got
through
>> every commit and gauge the risk/benefit, see if it applies, if it
passes the
>> tests…
> 
> Indeed!
> 
> However, during our "commit message" discussion [1], the
consensus was
> that too many rules means everyone will forget one or the other and
> the result will be worse. So we have to be careful with what we ask.
I didn’t mean to imply that we need to change/enforce anything, I’m aware of the
trade offs. I was just pointing out some IMO relevant experience in response to
‘just choose an idles time for branching and you’ve got no issue’. We rely on
people’s goodwill to write nice commit messages — this is not going to change —
and my only goal was to remind readers why clear commit messages are important
(ie. I know about the guidelines, it would be nice if more people followed
them).

Fred
> 
>> Anything that helps tracking that a particular commit need followup
commits
>> to be fully functional would be a win. I think just mentioning the
original
>> commit in the followups is a minimum (then you can have tooling to
relate
>> the commits), but I don’t see a way to enforce this.
> 
> The commit message guideline [2] already says that:
> 
> * If the commit is a bug fix on top of another recently committed
> patch, or a revert or reapply of a patch, include the svn revision
> number of the prior related commit.
> 
> 
>> It’s not always easy to distinguish between the first four, and you
really
>> want to understand it before cherry-picking in a long living branch.
> 
> Right now [2], we already have [TAG]s, PR mentions, and NFC flags to
> account for most of what you want, but we do not have the set { opt /
> codegen / crash } for what the fix is.
> 
> It is an interesting idea, and I try to do that myself as free text as
> often as possible. But even if we add that, it still will be
> considered "guideline", not a "rule", and therefore
candidate for
> being ignored like all the others.
> 
> I personally like rules, and wouldn't mind some git hooks on commit
> messages, but the risk of false positives and maintenance of hooks is
> one that we cannot ignore. The alternatives are to either close down
> the commit access like the GCC folks do, or to impose a culture of
> backlash like the Kernel folks do.
> 
> That was discussed [1] and the consensus is that we prefer to have
> guidelines and keep reminding people to be responsible.
> 
> cheers,
> --renato
> 
> [1] http://lists.llvm.org/pipermail/llvm-dev/2014-September/077089.html
> [2] http://llvm.org/docs/DeveloperPolicy.html#commit-messages