"No new changes", is anyone working to fix it?

109 views
Skip to first unread message

Johan Björk

unread,
Aug 31, 2011, 8:15:37 AM8/31/11
to Repo and Gerrit Discussion
Hi guys,

We're having a workflow where the developers have direct push access to most branches, and a few has codereview restrictions. We quite often hit the problem where the developer wants to push his changes into a codereview branch from a direct-push branch. Gerrit refuses to accept the commits with 'no new changes'.

There was a discussion about a year ago [1], but I haven't found any more info after that.

Anyone done any work on this? If we build it, would anyone be interested?

Thanks
/Johan

Luthander, Fredrik

unread,
Aug 31, 2011, 9:04:46 AM8/31/11
to Johan Björk, Repo and Gerrit Discussion

Hi there!

 

Shouldn't it work if you cherry-pick the change and then remove the Change-ID line(with a git commit --amend), thus generating a new Change-ID line?

That gives Gerrit the signal to make a new change for review when pushed to the second branch.

 

--

Best regards,

    Fredrik Luthander

Sony Ericsson Mobile Communications AB

Brad Larson

unread,
Aug 31, 2011, 2:11:19 PM8/31/11
to Repo and Gerrit Discussion
Like Fredrik said, I don't think this is actually a bug. A Change-Id
line indicates that this patch set belongs to a change in Gerrit, and
the change it is pointing at has already been merged. You need to
switch to a new change-id if you are uploading a new Gerrit change.

Perhaps the error wording could be improved in this case. I'm sure a
patch to address that would be appreciated.

Brad


On Aug 31, 8:04 am, "Luthander, Fredrik"
<Fredrik.Luthan...@sonyericsson.com> wrote:
> Hi there!
>
> Shouldn't it work if you cherry-pick the change and then remove the Change-ID line(with a git commit --amend), thus generating a new Change-ID line?
> That gives Gerrit the signal to make a new change for review when pushed to the second branch.
>
> --
> Best regards,
>     Fredrik Luthander
> Sony Ericsson Mobile Communications AB
>
> From: repo-d...@googlegroups.com [mailto:repo-d...@googlegroups.com] On Behalf Of Johan Björk
> Sent: onsdag den 31 augusti 2011 14:16
> To: Repo and Gerrit Discussion
> Subject: "No new changes", is anyone working to fix it?
>
> Hi guys,
>
> We're having a workflow where the developers have direct push access to most branches, and a few has codereview restrictions. We quite often hit the problem where the developer wants to push his changes into a codereview branch from a direct-push branch. Gerrit refuses to accept the commits with 'no new changes'.
>
> There was a discussion about a year ago [1], but I haven't found any more info after that.
>
> Anyone done any work on this? If we build it, would anyone be interested?
>
> Thanks
> /Johan
>
> [1]:http://groups.google.com/group/repo-discuss/browse_thread/thread/35eb...

Johan Björk

unread,
Aug 31, 2011, 3:22:16 PM8/31/11
to Brad Larson, Repo and Gerrit Discussion
On Wed, Aug 31, 2011 at 8:11 PM, Brad Larson <bkla...@gmail.com> wrote:
Like Fredrik said, I don't think this is actually a bug.  A Change-Id
line indicates that this patch set belongs to a change in Gerrit, and
the change it is pointing at has already been merged.  You need to
switch to a new change-id if you are uploading a new Gerrit change.

Well, maybe it's not a bug, but it doesn't allow for our workflow.

We have a few "important" branches where we require codereview. For other branches, developers are free to collaborate and do what they want.
We may, or may not, set a change id in the commits that go directly into a "private" branch, (but gerrit will autoassign one).
*the change has never shown up in the web-ui*
When the developer is ready to submit it to one of the "important" branches (which might be the first and only commit), they are
1) unable to push to refs/heads/<important branch> because the policy don't allow them to do direct pushes
2) can't push it to review because gerrit will say there are no changes.


I'm still not certain how this should work in a full scale, ie, when should a new review be created? I'm tempting to lean towards the simple case:

If a change is pushed to refs/for/X and the commit is not in X, a new review request will be created. (Even if it has been reviewed before, but aimed for a different branch)
I guess extending the UI that can link back to the previous reviews might be handy here. While it seems kind of neat to have it all in one review, I am afraid the UI would be too cluttered.


/Johan

Brad Larson

unread,
Aug 31, 2011, 4:17:42 PM8/31/11
to Repo and Gerrit Discussion


On Aug 31, 2:22 pm, Johan Björk <p...@spotify.com> wrote:
> On Wed, Aug 31, 2011 at 8:11 PM, Brad Larson <bklar...@gmail.com> wrote:
> > Like Fredrik said, I don't think this is actually a bug.  A Change-Id
> > line indicates that this patch set belongs to a change in Gerrit, and
> > the change it is pointing at has already been merged.  You need to
> > switch to a new change-id if you are uploading a new Gerrit change.
>
> > Well, maybe it's not a bug, but it doesn't allow for our workflow.
>
> We have a few "important" branches where we require codereview. For other
> branches, developers are free to collaborate and do what they want.
> We may, or may not, set a change id in the commits that go directly into a
> "private" branch, (but gerrit will autoassign one).
> *the change has never shown up in the web-ui*
> When the developer is ready to submit it to one of the "important" branches
> (which might be the first and only commit), they are
> 1) unable to push to refs/heads/<important branch> because the policy don't
> allow them to do direct pushes
> 2) can't push it to review because gerrit will say there are no changes.

Is there a reason your developers can't 'git commit --amend' and
remove the Change-Id line from the commit message? Then everything
will work fine.

I understand the confusion that Change-Ids can't be reused even if
they aren't used in a Gerrit change, but right now any change pushed
through Gerrit (even if it bypasses review) has its Change-Id
registered.

Johan Björk

unread,
Aug 31, 2011, 4:29:05 PM8/31/11
to Brad Larson, Repo and Gerrit Discussion
On Wed, Aug 31, 2011 at 10:17 PM, Brad Larson <bkla...@gmail.com> wrote:


On Aug 31, 2:22 pm, Johan Björk <p...@spotify.com> wrote:
> On Wed, Aug 31, 2011 at 8:11 PM, Brad Larson <bklar...@gmail.com> wrote:
> > Like Fredrik said, I don't think this is actually a bug.  A Change-Id
> > line indicates that this patch set belongs to a change in Gerrit, and
> > the change it is pointing at has already been merged.  You need to
> > switch to a new change-id if you are uploading a new Gerrit change.
>
> > Well, maybe it's not a bug, but it doesn't allow for our workflow.
>
> We have a few "important" branches where we require codereview. For other
> branches, developers are free to collaborate and do what they want.
> We may, or may not, set a change id in the commits that go directly into a
> "private" branch, (but gerrit will autoassign one).
> *the change has never shown up in the web-ui*
> When the developer is ready to submit it to one of the "important" branches
> (which might be the first and only commit), they are
> 1) unable to push to refs/heads/<important branch> because the policy don't
> allow them to do direct pushes
> 2) can't push it to review because gerrit will say there are no changes.

Is there a reason your developers can't 'git commit --amend' and
remove the Change-Id line from the commit message?  Then everything
will work fine.

That wouldn't be possible if there is no Change-Id line (ie, they did not have the hook installed). + it would change the commit, so if they have a dev branch tracking <important branch>, they would end up with two copies of the same commit.

Brad Larson

unread,
Aug 31, 2011, 4:44:44 PM8/31/11
to Repo and Gerrit Discussion, Shawn Pearce


On Aug 31, 3:29 pm, Johan Björk <p...@spotify.com> wrote:
Ah, sorry for my misunderstanding.

This is a tricky situation - if your dev branch has pushed a commit
bypassing review, and you want to pull up your important branch, you
are asking gerrit to do a change review of a commit which has already
been pushed. That isn't possible. The only (current) solution is to
amend the commit, which like you mentioned will result in duplicate
commits - one on the dev branch and one in the important branch.

Another solution is to create a merge commit which merges the dev
branch (with its one commit) into the important branch, and review
that. There are a lot of downsides to this approach, but it avoids
the duplicate commits.

Maybe Shawn has some other ideas.

Brad
Reply all
Reply to author
Forward
0 new messages