[OpenStack-Infra] BUG: [Cosmetic] Commiting a gerrit ChangeId before the commit hook was installed

Strainu strainu10 at gmail.com
Fri Jan 17 23:38:27 UTC 2014


Perhaps I haven't been clear enough: the commit already had a change
ID, added manually, so with or without the hook it would have been
attached to the correct review.

In this case, the hook will actually do nothing, making the current
wording of the message confusing IMO. My suggestion was [1] to change
the mesage to "Amending the commit to add a gerrit change id if none
is available." or something similar.

Strainu

[1] https://bugs.launchpad.net/git-review/+bug/1270301

2014/1/18 Jerry Xinyu Zhao <xyzjerry at gmail.com>:
> I think if you hadn't installed the commit hook for generating change ID,
> the commit indeed wouldn't have included a change ID, which is necessary for
> referencing the change when you submit a patch over it. There is nothing
> wrong with the message.  git review tool will install the hook and add a
> change ID for you automatically(a new feature of recent git-review release).
>
>
>
> On Fri, Jan 17, 2014 at 1:10 PM, Strainu <strainu10 at gmail.com> wrote:
>>
>> 2014/1/17 Jonathan Nieder <jrnieder at gmail.com>:
>> > Hi,
>> >
>> > Strainu wrote:
>> >
>> >> strainu at emily:~/core> git review -f
>> >> Creating a git remote called "gerrit" that maps to:
>> >>         ssh://strainu@gerrit.wikimedia.org:29418/pywikibot/core.git
>> >> Your change was committed before the commit hook was installed.
>> >> Amending the commit to add a gerrit change id.
>> >>
>> >> At this point I ended the transaction, as I was confused by the last
>> >> message: I was afraid the ChangeId would have changed, causing the
>> >> patch to be attached to another review.
>> >>
>> >> I think git should not show this message if the change description
>> >> already has a change id
>> >
>> > This message doesn't come from git.  It comes from the git-review
>> > tool (in git_review/cmd.py), so cc-ing the authors in case they
>> > have thoughts on that.
>>
>> Thanks for clarifying that. I'll log a bug on launchpad then.
>>
>> Strainu
>>
>> _______________________________________________
>> OpenStack-Infra mailing list
>> OpenStack-Infra at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra
>
>



More information about the OpenStack-Infra mailing list