Re: [workflow] Automatically close bug report when a patch is committed

[Giovanni Biscuolo]

Hello Simon,

thank you for havig listed possible troubles.

Before commenting, just let me repeat that we are _copying_ the
'Change-Id' idea (and related possible implementation issues) from
Gerrit:

https://gerrit-review.googlesource.com/Documentation/user-changeid.html

This means that Somewhere™ in our documentation we should start
explaining that:

--8<---------------cut here---------------start------------->8---

Our code review system needs to identify commits that belong to the same
review.  For instance, when a proposed patch needs to be modified to
address the comments of code reviewers, a second version of that patch
can be sent to guix-patches@gnu.org.  Our code review system allows
attaching those 2 commits to the same change, and relies upon a
Change-Id line at the bottom of a commit message to do so.  With this
Change-Id, our code review system can automatically associate a new
version of a patch back to its original review, even across cherry-picks
and rebases.

--8<---------------cut here---------------end--------------->8---

In other words, 'Change-Id' is /just/ metadata automatically added to
help in code review **tracking**, specificcally helping "across
cherry-picks and rebases" [1]

Sorry if I'm repeating things probably already understood!

Simon Tournier <zimon.toutoune@gmail.com> writes:

[...]

>> Please can you expand what troubles do you see in automatically adding
>> 'Change-Id:' using a hook-commit-msg like
>> https://gerrit-review.googlesource.com/Documentation/cmd-hook-commit-msg.html
>> ?
>
> 1. The hook must be installed.

AFAIU a hook is already installed when configuring for contribution.

If this is stil not properly documented it will be fixed.

> 2. The hook must not be in conflict with user configuration.

I guess you mean not in conflict with other locally installed git hooks:
since AFAIU we **already** have a locally installed git hook, this is
already a requirement and this is something the user (contributor)
should be aware of.

If this is stil not properly documented it will be fixed.

> 3. The generated Change-Id string can be mangled by some user unexpected
>    action.

Contributors and committers should not delete or change and already
existing 'Change-Id', this will be documented.

> Many things can rail out on user side.  For an example, base-commit is
> almost appearing systematically in submitted patches almost 3 years
> later.

I don't understand how this could impact the addition of the
patch-tracking metadata named 'Change-Id'

> The patches of some submissions are badly formatted.  Etc.

I don't understand what is the problem of having a 'Change-Id' (in
commit messages) in badly formatted patch submissions.

> Whatever the implementation, I am not convinced that the effort is worth
> the benefits.

OK, I'm sorry

> And I am not convinced it will help in closing the submissions when
> the patches have already been applied.

OK, I'm sorry

> That’s said, I am not against the proposal.  I just have mixed feelings
> and before deploying I strongly suggest to review if the proposal covers
> the intent.

OK, thank you for your suggestion.

Happy hacking! Gio'


[1] AFAIU 'Change-Id' can even track different versions of patches (that
by design are from commits in the same branch, properly rebased as
needed) sent by mistake via **different bug reports**, this also means
that different bug reports containing the same 'Change-Id' are _surely_
linked togheter.

-- 
Giovanni Biscuolo

Xelera IT Infrastructures

signature.asc
Description: PGP signature