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

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

> On May 17, 2016, at 10:43 AM, Hans Wennborg via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> 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.
FWIW I really prefer:

```
[component/target] Fix foo in the bar function

This commit attempts to address fix bla that happens because bla
   ...

http://llvm.org/PRxxx
(or PRxxx).
```

I.e. I don't really about the format of the link to the PR (even though it
may be clickable in various tools), but I care about a meaningful self-contained
*short* title (i.e. first line). For instance any tools I'm using to look at
our history are designed for that, here is a sample of what the history looks
like for me right now (pretty good):

2016-05-17 e5e8f5011002 (Cameron Desrochers): [PCH] Fixed bug with preamble
invalidation when overridden files change
2016-05-17 053db7298e39 (Filipe Cabecinhas): Revert "[X86] Add immediate
range checks for many of the builtins."
2016-05-17 c5e5ba39f43a (Petar Jovanovic): [Mips] Set mips32 as default CPU for
MIPS32 Android
2016-05-17 86786f5e688f (Alexey Bataev): [OPENMP] Pass scalar firstprivate vars
by value.
2016-05-17 3d656bfed4c5 (Martin Probst): clang-format: [JS] simplify logic by
parsing forward.
2016-05-17 06f267e2330f (Martin Probst): clang-format: [JS] fix template string
width counting.
2016-05-17 f5839f563d70 (Craig Topper): [AVX512] Add parentheses around macro
arguments in AVX512F intrinsics. Remove leading underscores from macro argument
names. Add explicit typecasts to all macro arguments and return values. And
finally reformat after all the adjustments.


Best,

-- 
Mehdi

Hans Wennborg via llvm-dev <llvm-dev at lists.llvm.org>
writes:
> On Mon, May 16, 2016 at 6:07 PM, Quentin Colombet via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>> 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.
+1. This would be a very useful thing to have, regardless of where the
rest of this thread goes.

On 17 May 2016 at 20:04, Justin Bogner via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>> 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.
>
> +1. This would be a very useful thing to have, regardless of where the
> rest of this thread goes.
I like this idea, too. But wouldn't this only work if the PR was in the
title?

--renato