This Bugzilla instance is deprecated, and most Eclipse projects now use GitHub or Eclipse GitLab. Please see the deprecation plan for details.
Bug 402791 - Wrong author on commit accepted via gerrit
Summary: Wrong author on commit accepted via gerrit
Status: RESOLVED INVALID
Alias: None
Product: Community
Classification: Eclipse Foundation
Component: Gerrit (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Eclipse Webmaster CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-08 16:55 EST by John Arthorne CLA
Modified: 2013-04-29 08:35 EDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description John Arthorne CLA 2013-03-08 16:55:19 EST
Brian de Alwis authored this change and pushed it to gerrit for review:

https://git.eclipse.org/r/#/c/10992/

I reviewed the change, and then click "Accept and Submit" from the gerrit review web page. Now when I look at the commit in master, it shows the author as me, but I didn't author the change:

http://git.eclipse.org/c/platform/eclipse.platform.runtime.git/commit/?id=3fac860e30ea84b7f513c1b4a83e27017abf020b

Shouldn't the author be... the author of the change submitted to gerrit??

Wondering if something is not configured properly, or if I did something wrong.
Comment 1 Paul Webster CLA 2013-03-08 17:31:45 EST
I've seen it keep the author correct, but that seems to be if the patch is already at the tip of master.  If it's not and Gerrit rebases the commit when it merges it in, it seems to replace the author.  But I'm not 100% sure, I'll test it later.

PW
Comment 2 Paul Webster CLA 2013-03-08 18:48:29 EST
See bug 401933 comment #4

Is this a per-project setting, or configured for all projects on our Gerrit instance?

PW
Comment 3 Paul Webster CLA 2013-03-08 18:56:40 EST
Our default is "Merge if Necessary", see http://gerrit.googlecode.com/svn/documentation/2.1/project-setup.html#submit_type

John, is this the kind of thing we want set to "Fast Forward Only"?

PW
Comment 4 Paul Webster CLA 2013-03-08 18:58:36 EST
(In reply to comment #0)
> Brian de Alwis authored this change and pushed it to gerrit for review:
> 
> https://git.eclipse.org/r/#/c/10992/
> 
> I reviewed the change, and then click "Accept and Submit" from the gerrit
> review web page. Now when I look at the commit in master, it shows the
> author as me, but I didn't author the change:

As Brian mentioned in the other bug, that's the merge commit (and his is still in there).

PW
Comment 5 John Arthorne CLA 2013-03-10 22:41:29 EDT
Yep sorry for the noise. I wasn't expecting a merge commit there.
Comment 6 John Arthorne CLA 2013-03-11 08:57:41 EDT
(In reply to comment #3)
> Our default is "Merge if Necessary", see
> http://gerrit.googlecode.com/svn/documentation/2.1/project-setup.
> html#submit_type
> 
> John, is this the kind of thing we want set to "Fast Forward Only"?
> 
> PW

This is probably not the place to discuss it, but gerrit doing auto-merge is kind of scary. It means Gerrit is releasing into master a combination of commits that has never been tested together. I usually avoid merge, but when it makes sense I like to at least test to ensure all the changes work together properly.
Comment 7 Markus Keller CLA 2013-04-26 09:20:45 EDT
> This is probably not the place to discuss it, but gerrit doing auto-merge is
> kind of scary. It means Gerrit is releasing into master a combination of
> commits that has never been tested together.

I agree that's scary. We should only allow safe commits ("Fast Forward Only"), like we already do without Gerrit.
Comment 8 Thanh Ha CLA 2013-04-26 10:26:08 EDT
(In reply to comment #7)
> > This is probably not the place to discuss it, but gerrit doing auto-merge is
> > kind of scary. It means Gerrit is releasing into master a combination of
> > commits that has never been tested together.
> 
> I agree that's scary. We should only allow safe commits ("Fast Forward
> Only"), like we already do without Gerrit.

I think a new bug should be opened listing all the repos that should be changed to "Fast Forward Only" if this is the case.
Comment 9 Dani Megert CLA 2013-04-29 08:35:46 EDT
(In reply to comment #8)
> (In reply to comment #7)
> > > This is probably not the place to discuss it, but gerrit doing auto-merge is
> > > kind of scary. It means Gerrit is releasing into master a combination of
> > > commits that has never been tested together.
> > 
> > I agree that's scary. We should only allow safe commits ("Fast Forward
> > Only"), like we already do without Gerrit.
> 
> I think a new bug should be opened listing all the repos that should be
> changed to "Fast Forward Only" if this is the case.

I suggest to do this for all Gerrit instances of the Eclipse top-level project. Let's discuss this in our next PMC meeting.