[llvm-dev] [RFC] Helping release management

Tue May 17 09:25:09 PDT 2016

Hi Renato,

Thanks for your feedbacks.

> On May 17, 2016, at 5:02 AM, Renato Golin <renato.golin at linaro.org> wrote:
> 
> On 17 May 2016 at 02:07, Quentin Colombet via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>> Basically, the high-level status is:
>> 1. Commits should state when they are fixes.
>> 2. Bugs should be tracked in a PR.
> 
> Yup.
> 
> 
>> None of these is a hard requirement but instead, best practices that we
>> should remind to the contributors.
>> For #1, I propose the attached patch for the commit message policy.
> 
> Your proposal goes against another policy, which is to leave the
> discussion to the bugzilla, since it's either already there, or should
> be there.

I do not see how the proposal goes against that policy. I certainly believe that the discussion should stay in the bugzilla, but I do not see why this is incompatible with the fact of summarizing some information in the commit message.
Basically what I tend to push for is self contained information for quick reference in the log. Then, if people want more information, they are encouraged to check the bugzilla.

> 
> Also, links can rot, PR numbers cannot. (hey, it rhymes!)

Yes, links can rot, though http:/llvm.org/PR# is relatively stable (at least it did not break with the last migration), plus the PR number is still here.
In other words, I believe that link adds a convenient way to access bugzilla while still providing the default information if the link rots.

> 
> 
>> For A, the idea is to have an automatic way to update the PRs (like resolved
>> and fixed with a revision number) when some keywords are set (like fixes
>> PR#). The idea with Diffusion is to have a field that marks the commit has
>> been a fix and that we could manually set if we forgot to set the keyword at
>> commit time.
> 
> A commit that "Fixes PR123" does not necessarily close it. There may
> be multiple commits, or we may wait until the buildbots are happy to
> close it (I certainly do).

I am fine with that definition :).

> 
> I think that the update here is simpler than that: Whenever a commit
> refers a PR, update the bug saying "commit rNNNN was marked as a fix
> to this bug".
> 
> People CC on the bug can then verify and act appropriately.

Works for me.

> 
> 
>> For B, assuming the PR or Diffusion becomes the way to keep trace of every
>> fixes, it is a matter of making easy to document that we are fixing a bug
>> even if a PR does not exist. Maybe it is possible to have some commit hooks
>> that given a keyword (e.g., fixes PR#New) file a PR with the commit message
>> and marks the PR as fixed (also patch the actual commit message to put the
>> PR number).
> 
> If you add some hyper-text markings to make it explicit, then creating
> a bug will be unequivocal. But if you try to guess by the description,
> then we'll end up with a flood of bugs (been there, done that).
> 
> Right now, "fixes PR" is our hyper-text marking, but it's only
> injective, you want something bijective. I can't think of anything
> that would have the same semantics whether you have a PR number or
> not.

I miss the point.
For existing PR we would have: fixes PR1234.
For new PR we would have: fixes PRNew, where PRNew is a keyword that means create a PR and put the actual number in place of PRNew.

> 
> I actually see this as an attempt to overcome the problem that people
> don't create enough PRs, by forcing them to write more on the commit
> message. For me, this is trying to solve the wrong problem.

Well, what I am trying to do is to make that automatic. I would be the first one annoyed if I have to file a PR before fixing any thing. If when I am fixing something, I just have to add “fixes PRNew”, that sounds easy enough to me :).

That being said, is your concern the fact that if we make that commit+PR creation easy, people would not file PR before hand?

> 
> I'm also strongly against any more regulation of the commit message,
> as this can create a culture of backlash when people don't, which can
> lead to a sour community.

You may be right, though I feel like people in the community were responding well when we bring to their attention that their commit message would have been more useful if they added this or that (like the link to the buildbot that broke when reverting).

I see this as an education step to bring all the contributions to the high standard we all love. Maybe I am naive and people would actually be against that and I would love to hear other opinions.

> 
> With my autistic hat on, I'd *love* a very strong set of rules and
> regulations. But when I wear my community hat, I realise that I'm
> probably the only one that would like my own rules, as are we all.
> 
> 
>> Now, regarding how this information could be used by release managers to
>> track what has been pulled in their release, I believe we would need to
>> build additional tools on top of that information, but having this
>> information is the priority IMHO.
> 
> This is the problem we were trying to solve in the first place, and I
> think we changed the priorities with the illusion that it would get us
> closer to the real problem. I don't think it will.
> 
> By writing a script that will *only* track PRs and the commits that
> refers to them, we can easily *drive* people to use the proper path
> with a carrot, not a stick.
> 
> If you file a bug (as per current community practices), and link your
> commits to it with a "fixes PR" message (as per current commit
> policy), we'll track your commit and make it a candidate for
> back-ports.
> 
> If you don't, you'll be summarily ignored. Have a nice day. After all,
> we can't baby sit everyone that doesn't want to follow the rules that
> already exist.

Sure, but I don’t see why you are opposing this two approaches: file a PR followed by a fix and file a PR with a fix.
Of course, people that don’t use the marking keywords will be out of the system, that I agree, but it seems to me that having automatic process like this will lower the bar for most people (assuming this is possible).

> 
> We could then use Bugzilla to mark to which release we'd like
> back-ported. We can use the "Version" field, or "Keywords". All of
> that easily automated.

Agree. Though this goes beyond what I am trying to solve: document fixes.

> 
> We can also only grab the list of bugs that have been closed as
> "Delivered", check the list of patches against it, add to the list,
> send to the release manager with the names of all the people that
> "fixed" that bug, and ask them for more information on possible fix
> ups and reverts.
> 
> With time, people will get on track, and less effort will be needed by
> release managers, because people will begin to add "PR" tags on
> reverts, fixups, etc. and all will fall into place without effort.
> 
> My tuppence.
> 
> --renato

Thanks again for your input, I think it helps clarifying what is at stake and how we plan to address it.

Cheers,
-Quentin