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

Filing a PR for *every* fix is overkill.  But filing a PR to express
"please merge this to the branch" (if there isn't already a PR)
seems reasonable.
--paulr

From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of Reid
Kleckner via llvm-dev
Sent: Thursday, May 05, 2016 10:09 AM
To: Quentin Colombet
Cc: llvm-dev
Subject: Re: [llvm-dev] [RFC] Helping release management

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160505/c480d3be/attachment-0001.html>

Hi,

Let me summarize the current status of the discussion to check and propose a
path forward.

* Summary *

Basically, the high-level status is:
1. Commits should state when they are fixes.
2. Bugs should be tracked in a PR.

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.


* What Is Next? *

Now, moving forward, we should investigate if it is possible to have:
A. Hooks on commit to automatically update some field in Diffusion/Bugzilla when
a fix is committed.
B. PRs automatically created for fixes that do not have a PR.

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.

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).

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.


* Feedback Needed *

- Is it possible to have commit hooks to set some fields in Diffusion/Bugzilla?
- Is it possible to have commit hooks to create PRs?
- What do people think of the attached patch?

Cheers,
-Quentin


> On May 5, 2016, at 11:47 AM, Robinson, Paul <paul.robinson at
sony.com> wrote:
> 
> Filing a PR for *every* fix is overkill.  But filing a PR to express
"please merge this to the branch" (if there isn't already a PR)
seems reasonable.
> --paulr
>   <>
> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of
Reid Kleckner via llvm-dev
> Sent: Thursday, May 05, 2016 10:09 AM
> To: Quentin Colombet
> Cc: llvm-dev
> Subject: Re: [llvm-dev] [RFC] Helping release management
>  
> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160516/827fa43d/attachment-0002.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: commitMessage.patch
Type: application/octet-stream
Size: 929 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160516/827fa43d/attachment-0001.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160516/827fa43d/attachment-0003.html>

> On May 16, 2016, at 6:07 PM, Quentin Colombet via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Hi,
> 
> Let me summarize the current status of the discussion to check and propose
a path forward.
> 
> * Summary *
> 
> Basically, the high-level status is:
> 1. Commits should state when they are fixes.
> 2. Bugs should be tracked in a PR.
> 
> 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.
> 
> 
> * What Is Next? *
> 
> Now, moving forward, we should investigate if it is possible to have:
> A. Hooks on commit to automatically update some field in Diffusion/Bugzilla
when a fix is committed.
> B. PRs automatically created for fixes that do not have a PR.
> 
> 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.
> 
> 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).
> 
> 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.
> 
> 
> * Feedback Needed *
> 
> - Is it possible to have commit hooks to set some fields in
Diffusion/Bugzilla?
> - Is it possible to have commit hooks to create PRs?
This is definitely possible, (we already have python script to interact between
our internal BT and Bugzilla), but "someone" has to do it though...
> - What do people think of the attached patch?
LGTM.

-- 
Mehdi

> 
> Cheers,
> -Quentin
> 
> <commitMessage.patch>
> 
>> On May 5, 2016, at 11:47 AM, Robinson, Paul <paul.robinson at
sony.com <mailto:paul.robinson at sony.com>> wrote:
>> 
>> Filing a PR for *every* fix is overkill.  But filing a PR to express
"please merge this to the branch" (if there isn't already a PR)
seems reasonable.
>> --paulr
>>   <>
>> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org
<mailto:llvm-dev-bounces at lists.llvm.org>] On Behalf Of Reid Kleckner
via llvm-dev
>> Sent: Thursday, May 05, 2016 10:09 AM
>> To: Quentin Colombet
>> Cc: llvm-dev
>> Subject: Re: [llvm-dev] [RFC] Helping release management
>>  
>> 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.
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160516/c9372ab1/attachment.html>

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.

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

> 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 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.

> 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 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.

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.

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.

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.

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

On Mon, May 16, 2016 at 6:07 PM, Quentin Colombet via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> Let me summarize the current status of the discussion to check and propose
a
> path forward.
>
> * Summary *
>
> Basically, the high-level status is:
> 1. Commits should state when they are fixes.
> 2. Bugs should be tracked in a PR.
>
> 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.
>
>
> * What Is Next? *
>
> Now, moving forward, we should investigate if it is possible to have:
> A. Hooks on commit to automatically update some field in Diffusion/Bugzilla
> when a fix is committed.
> B. PRs automatically created for fixes that do not have a PR.
>
> 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.
Like someone else pointed out, the PR shouldn't necessarily be marked
fixed because there's a commit referring to it.

The way I'd propose is simply a commit hook that adds a message to the
bug: "Commit rXXX refers to this bug:\n\n(quote of commit message)".

Note that this would also fire when a commit referring to a PR gets
reverted (assuming the reverting commit doesn't botch the commit
message), etc., which is super useful for those following along on the
bug's cc list.
> * Feedback Needed *
>
> - Is it possible to have commit hooks to set some fields in
> Diffusion/Bugzilla?
Yes, and since our SVN repository and Bugzilla are on the same server
it shouldn't be hard. I've been meaning to look into this, but
didn't
get very far yet. There are some links here:
https://wiki.mozilla.org/Bugzilla:Addons#Integration_with_Source_Code_Management_programs
and e.g. this looks pretty simple:
http://www.telegraphics.com.au/svn/svn_bz/trunk/svn_bz_append.pl
> - What do people think of the attached patch?
FWIW, I'd prefer just "Fix foo in the bar function (PRxxx)" to
"Fix
foo in the bar function (http://llvm.org/PRxxx)". But I don't feel
strongly enough to object if others find the URL super useful.

Cheers,
Hans