You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Patrick Mueller <pm...@gmail.com> on 2012/10/10 17:21:36 UTC
marking pull request committer correctly
wops. Made a boo-boo.
If you look at this commit:
https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-ios.git;a=commit;h=4d322c0b786e96aaac15ee6fbea5a832ac40ea43
you'll notice that Marcel is marked as the author and committer. But I
committed the code, since Marcel isn't a committer. That's the boo-boo.
Oh, and Marcel is (not yet) an Apache Cordova committer, but has
contributed some other code.
I followed an easy path to get the code in, via github pull request:
git co master
git pull
git pull https://github.com/cmarcelk/incubator-cordova-ios cb-1473
... test the code ...
git push
Or something like that. Apparently that wasn't enough to trigger getting
the committer changed to myself.
We do have directions in the wiki about how to handle pull requests:
http://wiki.apache.org/cordova/CommitterWorkflow
I suspect there is an easier recipe without having to do the `git remote
add`, which is silly. Something like:
- create branch
- pull github pull request
- merge into master
Thinking that the merge will change the committer to the actual committer
instead of contributor.
Thoughts?
And two questions:
1) Do we have an way to change the message that gets sent to the dev list
from github pull request, like this one:
http://markmail.org/message/6nmxtszeb4nudg4j
Perhaps replace the instructions with a link to the CommitterWorkflow page.
2) Do we have a way of preventing this from happening? Have git to do
check on pushes to ensure the git committer is a valid Apache Cordova
committer.
--
Patrick Mueller
http://muellerware.org
Re: marking pull request committer correctly
Posted by Andrew Grieve <ag...@chromium.org>.
I think the committer field is just the user who created the commit, where
the commit is defined by it's hash. If you change the hash in any way, then
the committer becomes you. E.g. it might be enough to do a git pull, then
do a git commit --amend and add change some whitespace in the change
description.
On Wed, Oct 10, 2012 at 8:13 PM, Patrick Mueller <pm...@gmail.com> wrote:
> On Wed, Oct 10, 2012 at 7:01 PM, Jukka Zitting <jukka.zitting@gmail.com
> >wrote:
>
> > Right. The repositories on git-wip-us automatically maintain an OOB
> > push log that ties each push to a specific @apache.org account,
> > regardless of what the included Git commit messages say.
> >
>
> Ah, cool. And I guess we can get to this ref-updates.log file if we need
> to, via infra.
>
>
> > On the "Committer" field in a Git commit message; I'm actually not
> > entirely sure if forcing it to be the same as the "Apache committer"
> > who pushes the changes to git-wip-us is a good idea. The "Committer"
> > field has a pretty specific technical meaning in Git and mixing that
> > with the somewhat different meaning that the *committer* term might
> > cause some confusion.
> >
>
> I think that's EXACTLY what you want: code contributor marked as author,
> Apache committer marked as committer.
>
> from: http://git-scm.com/book/ch2-3.html
>
> You may be wondering what the difference is between author and
> committer. The author is the person who originally wrote the patch,
> whereas the committer is the person who last applied the patch.
>
> And that's what struck me as weird; Marcel should have been the author, and
> I should have been the committer.
>
>
> > If we want to mention in the commit message the
> > Apache committer who accepted a given pull request and pushed those
> > commits to the canonical repository, using the Signed-off-by mechanism
> > might be a better alternative. See also some earlier infra-dev@
> > discussion in http://markmail.org/message/ljezsfv2mvzg7gll.
> >
>
> I'm guessing the Signed-off-by thing is a magic comment we'd put in the
> commit? Actually, wouldn't have helped in this case, since the commit
> fast-forwarded, I didn't even get a chance to add a comment! Super
> efficient!
>
> Anyway, definitely not interested in something like a signed-off-by
> process. This is such a minor issue, that I don't think worth trying to
> "fix". Especially since for pull requests and other non-committer
> contributions, there's likely to be plenty of other context around (Jira
> bugs, ml conversations), that I'm guessing we can always figure out what
> happened anyway.
>
> Although I will likely do a bit more sniffing around for my next pull
> request!
>
> --
> Patrick Mueller
> http://muellerware.org
>
Re: marking pull request committer correctly
Posted by Patrick Mueller <pm...@gmail.com>.
On Wed, Oct 10, 2012 at 7:01 PM, Jukka Zitting <ju...@gmail.com>wrote:
> Right. The repositories on git-wip-us automatically maintain an OOB
> push log that ties each push to a specific @apache.org account,
> regardless of what the included Git commit messages say.
>
Ah, cool. And I guess we can get to this ref-updates.log file if we need
to, via infra.
> On the "Committer" field in a Git commit message; I'm actually not
> entirely sure if forcing it to be the same as the "Apache committer"
> who pushes the changes to git-wip-us is a good idea. The "Committer"
> field has a pretty specific technical meaning in Git and mixing that
> with the somewhat different meaning that the *committer* term might
> cause some confusion.
>
I think that's EXACTLY what you want: code contributor marked as author,
Apache committer marked as committer.
from: http://git-scm.com/book/ch2-3.html
You may be wondering what the difference is between author and
committer. The author is the person who originally wrote the patch,
whereas the committer is the person who last applied the patch.
And that's what struck me as weird; Marcel should have been the author, and
I should have been the committer.
> If we want to mention in the commit message the
> Apache committer who accepted a given pull request and pushed those
> commits to the canonical repository, using the Signed-off-by mechanism
> might be a better alternative. See also some earlier infra-dev@
> discussion in http://markmail.org/message/ljezsfv2mvzg7gll.
>
I'm guessing the Signed-off-by thing is a magic comment we'd put in the
commit? Actually, wouldn't have helped in this case, since the commit
fast-forwarded, I didn't even get a chance to add a comment! Super
efficient!
Anyway, definitely not interested in something like a signed-off-by
process. This is such a minor issue, that I don't think worth trying to
"fix". Especially since for pull requests and other non-committer
contributions, there's likely to be plenty of other context around (Jira
bugs, ml conversations), that I'm guessing we can always figure out what
happened anyway.
Although I will likely do a bit more sniffing around for my next pull
request!
--
Patrick Mueller
http://muellerware.org
Re: marking pull request committer correctly
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Thu, Oct 11, 2012 at 1:11 AM, Patrick Mueller <pm...@gmail.com> wrote:
> I'll add some of these tool-y bits to the CommitterWorkflow page. But I
> guess if we aren't actively checking the committer/author bits in a git
> hook, folks can put garbage in anyway.
Right. The repositories on git-wip-us automatically maintain an OOB
push log that ties each push to a specific @apache.org account,
regardless of what the included Git commit messages say.
So it's not too big an issue if the committer/author bits aren't
absolutely correct, but having them in good shape of course makes
things clearer when later on browsing the version history.
On the "Committer" field in a Git commit message; I'm actually not
entirely sure if forcing it to be the same as the "Apache committer"
who pushes the changes to git-wip-us is a good idea. The "Committer"
field has a pretty specific technical meaning in Git and mixing that
with the somewhat different meaning that the *committer* term might
cause some confusion. If we want to mention in the commit message the
Apache committer who accepted a given pull request and pushed those
commits to the canonical repository, using the Signed-off-by mechanism
might be a better alternative. See also some earlier infra-dev@
discussion in http://markmail.org/message/ljezsfv2mvzg7gll.
BR,
Jukka Zitting
Re: marking pull request committer correctly
Posted by Patrick Mueller <pm...@gmail.com>.
It appears one trick is that you can use the repo name in a pull. Here are
the instructions from the email sent out from the pull request:
http://markmail.org/message/6nmxtszeb4nudg4j
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cmarcelk/incubator-cordova-ios cb-1473
I did some experiments, and it appears that two different things happen,
depending on whether the pull is a fast-forward or not - it WAS in my case.
If it's a fast-forward, it must just leave the author/committer alone - no
actual new commit was created in this case. If it's not a fast-forward,
then it's a merge, and both the author/committer get set to ME! oh dear. :-)
git log will show you both author/committer if invoked as:
git log --pretty=full
Trolling around the web a bit, apparently you can use
git commit --author="whoever"
Presumably, you might use this in a commit --amend --author="whoever" AFTER
you pull/merge/rebase, but BEFORE you push to apache git. Of course, you
should NEVER amend a commit which you have already pushed.
I'll add some of these tool-y bits to the CommitterWorkflow page. But I
guess if we aren't actively checking the committer/author bits in a git
hook, folks can put garbage in anyway.
--
Patrick Mueller
http://muellerware.org
Re: marking pull request committer correctly
Posted by Filip Maj <fi...@adobe.com>.
Yeh I use
Git checkout -b somebranch name remotes/REMOTENAME/REMOTEBRANCH
On 10/10/12 12:24 PM, "Michal Mocny" <mm...@chromium.org> wrote:
>I was surprised that those instructions worked until I read what git
>checkout NAME/branch does.
>
>I found this:
>http://stackoverflow.com/questions/1783405/git-checkout-remote-branch
>which
>advises the syntax:
>
>git checkout -b branch NAME/branch
>
>which saves one command, I think.
>
>Either way, I am not sure how to avoid adding the remote in the first
>place.
>
>
>On Wed, Oct 10, 2012 at 2:58 PM, Andrew Grieve <ag...@chromium.org>
>wrote:
>
>> Interesting point you bring up.
>>
>> According to:
>>
>>
>>http://stackoverflow.com/questions/12296255/git-pull-rebase-changes-the-c
>>ommiter-name-to-my-name
>>
>> Doing a rebase will update the committer field. I like to do these
>>anyways
>> since it will avoid the extra "Merge blah blah" commit in the history.
>>
>> I don't know how to do this without using a remote though...
>> git remote add NAME URL
>> git checkout NAME/branch
>> git checkout -b mybranch
>> git rebase master
>> git checkout master
>> git merge --ff-only mybranch
>> git push apache master
>>
>> Anyone else have ideas?
>>
>>
>>
>> On Wed, Oct 10, 2012 at 11:27 AM, Patrick Mueller <pm...@gmail.com>
>> wrote:
>>
>> > On Wed, Oct 10, 2012 at 11:21 AM, Patrick Mueller <pm...@gmail.com>
>> > wrote:
>> >
>> > > wops. Made a boo-boo.
>> > >
>> > >
>> > Another interesting tidbit - the email on the commits list was
>>addressed
>> as
>> > from me, which is helpful. And confusing, I guess. :-)
>> >
>> > http://markmail.org/message/rvt2sm6pszgbtpli
>> >
>> > --
>> > Patrick Mueller
>> > http://muellerware.org
>> >
>>
Re: marking pull request committer correctly
Posted by Michal Mocny <mm...@chromium.org>.
I was surprised that those instructions worked until I read what git
checkout NAME/branch does.
I found this:
http://stackoverflow.com/questions/1783405/git-checkout-remote-branch which
advises the syntax:
git checkout -b branch NAME/branch
which saves one command, I think.
Either way, I am not sure how to avoid adding the remote in the first place.
On Wed, Oct 10, 2012 at 2:58 PM, Andrew Grieve <ag...@chromium.org> wrote:
> Interesting point you bring up.
>
> According to:
>
> http://stackoverflow.com/questions/12296255/git-pull-rebase-changes-the-commiter-name-to-my-name
>
> Doing a rebase will update the committer field. I like to do these anyways
> since it will avoid the extra "Merge blah blah" commit in the history.
>
> I don't know how to do this without using a remote though...
> git remote add NAME URL
> git checkout NAME/branch
> git checkout -b mybranch
> git rebase master
> git checkout master
> git merge --ff-only mybranch
> git push apache master
>
> Anyone else have ideas?
>
>
>
> On Wed, Oct 10, 2012 at 11:27 AM, Patrick Mueller <pm...@gmail.com>
> wrote:
>
> > On Wed, Oct 10, 2012 at 11:21 AM, Patrick Mueller <pm...@gmail.com>
> > wrote:
> >
> > > wops. Made a boo-boo.
> > >
> > >
> > Another interesting tidbit - the email on the commits list was addressed
> as
> > from me, which is helpful. And confusing, I guess. :-)
> >
> > http://markmail.org/message/rvt2sm6pszgbtpli
> >
> > --
> > Patrick Mueller
> > http://muellerware.org
> >
>
Re: marking pull request committer correctly
Posted by Andrew Grieve <ag...@chromium.org>.
Interesting point you bring up.
According to:
http://stackoverflow.com/questions/12296255/git-pull-rebase-changes-the-commiter-name-to-my-name
Doing a rebase will update the committer field. I like to do these anyways
since it will avoid the extra "Merge blah blah" commit in the history.
I don't know how to do this without using a remote though...
git remote add NAME URL
git checkout NAME/branch
git checkout -b mybranch
git rebase master
git checkout master
git merge --ff-only mybranch
git push apache master
Anyone else have ideas?
On Wed, Oct 10, 2012 at 11:27 AM, Patrick Mueller <pm...@gmail.com> wrote:
> On Wed, Oct 10, 2012 at 11:21 AM, Patrick Mueller <pm...@gmail.com>
> wrote:
>
> > wops. Made a boo-boo.
> >
> >
> Another interesting tidbit - the email on the commits list was addressed as
> from me, which is helpful. And confusing, I guess. :-)
>
> http://markmail.org/message/rvt2sm6pszgbtpli
>
> --
> Patrick Mueller
> http://muellerware.org
>
Re: marking pull request committer correctly
Posted by Patrick Mueller <pm...@gmail.com>.
On Wed, Oct 10, 2012 at 11:21 AM, Patrick Mueller <pm...@gmail.com> wrote:
> wops. Made a boo-boo.
>
>
Another interesting tidbit - the email on the commits list was addressed as
from me, which is helpful. And confusing, I guess. :-)
http://markmail.org/message/rvt2sm6pszgbtpli
--
Patrick Mueller
http://muellerware.org