Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Wikipedia: Wikitech

Git + Gerrit is a toughy

 

 

Wikipedia wikitech RSS feed   Index | Next | Previous | View Threaded


otto at wikimedia

Feb 17, 2012, 5:47 PM

Post #1 of 11 (1278 views)
Permalink
Git + Gerrit is a toughy

Hi all!

And here's another hi: Hi! This is my first post to this list, so here is a quick intro in case you missed the other ones. I'm Andrew Otto, an engineering on the new Analytics team. I'm working with David Schoonover (new hire as well), Fabian Kaelin, and Diederik van Liere. Right now we're working on some prototypes for the a WikiMedia report card.

I think we are the first team that is doing active work in git using Gerrit, and Robla asked me to reach out here to describe our experiences and ask for help. We're struggling right now to be productive using Gerrit (I spent 3 hours today just trying to merge a branch), but it could be do to our lack of experience with it. There have been a couple of emails bouncing around to Ryan Lane and Roan, but it might be more productive if I made this conversation more visible here. I'll start with some questions.

1. Will Gerrit allow us to create branches without using the web GUI, and without having to be a Gerrit admin for a project?

One of the points of using git is to be able to create branches at will. We're finding this very difficult right now, not only because creating requires GUI admin access, but because of other reasons explained below.

2. Do I need to rebase every time I push for review?

I don't quite understand what is going on here. I've installed git-review and am using this to push to git. It does a rebase by default. I'm not sure if I should be turning that off or not. Rebases seem like a bad idea unless you really need to do them. I think git-review is doing a rebase by default so it can squash all of your local commits into one big review commit before pushing. Yuck! This would surely mean fewer commits to review in Gerrit, but it destroys real the history. It is making git work more like subversion, where you just work locally until everything is good and then have one big commit. I should be able to commit often and be able to share my commits with other developers before having everything reviewed.

3. How does Gerrit handle merges? Do all merge commits need to be re-approved?

4. What should I do in the following situation?

I have a branch I recently made from master. I've made some changes and pushed them to gerrit. My changes have been approved. Now I want to sync master into my branch. I do

git merge master

Then resolve any conflicts and commit. How should I push these changes? The commits that make up the merge have already been approved in gerrit on the master branch. Do I need to push for review using git-review? They've already been approved, so I would think not. But gerrit will currently not allow me to push without using git-review (is that because the commits need a Change-Id?).

Since gerrit doesn't let me do a regular git push to push my master merge to the remote branch I am tracking, I do git-review. This does rebase by default, so for some reason I am stuck having to resolve every single commit that was made to master in order to get the merge to push. This takes quite a while, but I did it, and once the interactive rebase was finished I was able to git-review to push the merge from master.

Great. Now I that my branch is in sync with master again, I want to merge it into master.

git checkout master
git merge my_branch

All good. Then what? Since I can't do just 'git push', I try git-review again. The same thing happens. I have to run through the whole interactive rebase routine and resolve each of my commits from my_branch manually. I do that, then run 'git-review' again. Now I get this error message:

remote: Hint: A potential Change-Id was found, but it was not in the footer of the commit message.
To ssh://otto [at] gerrit:29418/analytics/reportcard.git
! [remote rejected] HEAD -> refs/for/master/master (missing Change-Id in commit message)
error: failed to push some refs to 'ssh://otto [at] gerrit:29418/analytics/reportcard.git'

Each of the commits I merged from my_branch come with their own Change-Id in the commit messages. But these commits are now merge commits (I think?), so they have information about the merge and any conflicts in the commit message below the original Change-Id. I think this is confusing Gerrit, because it doesn't see the Change-Id in the footer.

Now I'm stuck, I'm really not sure how to push anymore. I want to get Diederik some of my changes, but I can't push them to master.


Thanks for the help everybody! It sounds like we in Analytics are the git+gerrit workflow Guinea pigs, eh? We're happy to fill this role, but SCMs are supposed to streamline and improve work flow, and right now Gerrit is being a big ol' nasty nancy. Help us iron this out so we can keep working!

- otto


http://ottomata.com
http://www.flickr.com/photos/OttomatonA
http://www.couchsurfing.org/people/otto


_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


roan.kattouw at gmail

Feb 18, 2012, 2:31 AM

Post #2 of 11 (1340 views)
Permalink
Re: Git + Gerrit is a toughy [In reply to]

On Sat, Feb 18, 2012 at 2:47 AM, Andrew Otto <otto [at] wikimedia> wrote:
> 2. Do I need to rebase every time I push for review?
>
> I don't quite understand what is going on here.  I've installed git-review and am using this to push to git.  It does a rebase by default.  I'm not sure if I should be turning that off or not.  Rebases seem like a bad idea unless you really need to do them. I think git-review is doing a rebase by default so it can squash all of your local commits into one big review commit before pushing. Yuck!  This would surely mean fewer commits to review in Gerrit, but it destroys real the history.  It is making git work more like subversion, where you just work locally until everything is good and then have one big commit.  I should be able to commit often and be able to share my commits with other developers before having everything reviewed.
>
Yes, you need to rebase before you push. The rebase does not exist to
squash multiple commits into one, but to ensure that your commit can
be merged cleanly. This fits the gated trunk model, but it looks like
you don't necessarily want to gate your working branch at all, just
your master. IMO you should ask Ryan to set up direct push access for
your working branches, so you can just git push into them directly,
bypassing review. You can then merge your branch into master, and
submit that merge commit for review.


> 3. How does Gerrit handle merges?  Do all merge commits need to be re-approved?
>
Yes.

> 4. What should I do in the following situation?
>
> I have a branch I recently made from master.  I've made some changes and pushed them to gerrit.  My changes have been approved.  Now I want to sync master into my branch.  I do
>
>  git merge master
>
Why did you merge master into your branch, rather than merging your
branch into master? That doesn't make much sense to me.

> Then resolve any conflicts and commit.  How should I push these changes?  The commits that make up the merge have already been approved in gerrit on the master branch.  Do I need to push for review using git-review?  They've already been approved, so I would think not.  But gerrit will currently not allow me to push without using git-review (is that because the commits need a Change-Id?).
>
Yes, you need to submit the merge commit for review. If some commits
don't have a Change-Id, git-review can't submit them, but I don't see
how that could be the case. You said the commits were already approved
in gerrit, *and* they don't have a Change-Id? Those things can't both
be true.

> Since gerrit doesn't let me do a regular git push to push my master merge to the remote branch I am tracking, I do git-review.
Perhaps you should ask for regular pushes to be allowed if you're not
using the review workflow for that branch, see also above.

> This does rebase by default, so for some reason I am stuck having to resolve every single commit that was made to master in order to get the merge to push.  This takes quite a while, but I did it, and once the interactive rebase was finished I was able to git-review to push the merge from master.
>
> Great.  Now I that my branch is in sync with master again, I want to merge it into master.
>
>  git checkout master
>  git merge my_branch
>
> All good.  Then what?  Since I can't do just 'git push', I try git-review again.  The same thing happens.  I have to run through the whole interactive rebase routine and resolve each of my commits from my_branch manually.  I do that, then run 'git-review' again.  Now I get this error message:
>
> remote: Hint: A potential Change-Id was found, but it was not in the footer of the commit message.
> To ssh://otto [at] gerrit:29418/analytics/reportcard.git
>  ! [remote rejected] HEAD -> refs/for/master/master (missing Change-Id in commit message)
> error: failed to push some refs to 'ssh://otto [at] gerrit:29418/analytics/reportcard.git'
>
> Each of the commits I merged from my_branch come with their own Change-Id in the commit messages.  But these commits are now merge commits (I think?), so they have information about the merge and any conflicts in the commit message below the original Change-Id.  I think this is confusing Gerrit, because it doesn't see the Change-Id in the footer.
>
There is a bug in git that causes merge commits to not automatically
get Change-IDs. After generating a merge commit, you need to run git
commit --amend , then save without changing anything. That makes sure
the commit-msg hook is run and the Change-ID is appended.

Roan

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


otto at wikimedia

Feb 21, 2012, 9:31 AM

Post #3 of 11 (1246 views)
Permalink
Re: Git + Gerrit is a toughy [In reply to]

> IMO you should ask Ryan to set up direct push access for
> your working branches
Cool, will do. Does Ryan read this list?

I'm not sure how MediaWiki should work, but maybe Gerrit should be set up like this by default? Either with a master + a production branch, or even just a master. Somewhere where only one branch needs review, and any other branches can be created and pushed at will, and review isn't needed until a merge to the master or production branch happens.

> Why did you merge master into your branch, rather than merging your
> branch into master? That doesn't make much sense to me.
Hmm, maybe I did this wrong then. Is this something I should never do with git at all, or just with this Gerrit workflow? Isn't merging from master into my branches part of a regular workflow? Shouldn't I be merging in the code from master all the time as I work?


Thanks Roan! We'll get this ironed out fo sho.

-otto



On Feb 18, 2012, at 2:31 AM, Roan Kattouw wrote:

> On Sat, Feb 18, 2012 at 2:47 AM, Andrew Otto <otto [at] wikimedia> wrote:
>> 2. Do I need to rebase every time I push for review?
>>
>> I don't quite understand what is going on here. I've installed git-review and am using this to push to git. It does a rebase by default. I'm not sure if I should be turning that off or not. Rebases seem like a bad idea unless you really need to do them. I think git-review is doing a rebase by default so it can squash all of your local commits into one big review commit before pushing. Yuck! This would surely mean fewer commits to review in Gerrit, but it destroys real the history. It is making git work more like subversion, where you just work locally until everything is good and then have one big commit. I should be able to commit often and be able to share my commits with other developers before having everything reviewed.
>>
> Yes, you need to rebase before you push. The rebase does not exist to
> squash multiple commits into one, but to ensure that your commit can
> be merged cleanly. This fits the gated trunk model, but it looks like
> you don't necessarily want to gate your working branch at all, just
> your master. IMO you should ask Ryan to set up direct push access for
> your working branches, so you can just git push into them directly,
> bypassing review. You can then merge your branch into master, and
> submit that merge commit for review.
>
>
>> 3. How does Gerrit handle merges? Do all merge commits need to be re-approved?
>>
> Yes.
>
>> 4. What should I do in the following situation?
>>
>> I have a branch I recently made from master. I've made some changes and pushed them to gerrit. My changes have been approved. Now I want to sync master into my branch. I do
>>
>> git merge master
>>
> Why did you merge master into your branch, rather than merging your
> branch into master? That doesn't make much sense to me.
>
>> Then resolve any conflicts and commit. How should I push these changes? The commits that make up the merge have already been approved in gerrit on the master branch. Do I need to push for review using git-review? They've already been approved, so I would think not. But gerrit will currently not allow me to push without using git-review (is that because the commits need a Change-Id?).
>>
> Yes, you need to submit the merge commit for review. If some commits
> don't have a Change-Id, git-review can't submit them, but I don't see
> how that could be the case. You said the commits were already approved
> in gerrit, *and* they don't have a Change-Id? Those things can't both
> be true.
>
>> Since gerrit doesn't let me do a regular git push to push my master merge to the remote branch I am tracking, I do git-review.
> Perhaps you should ask for regular pushes to be allowed if you're not
> using the review workflow for that branch, see also above.
>
>> This does rebase by default, so for some reason I am stuck having to resolve every single commit that was made to master in order to get the merge to push. This takes quite a while, but I did it, and once the interactive rebase was finished I was able to git-review to push the merge from master.
>>
>> Great. Now I that my branch is in sync with master again, I want to merge it into master.
>>
>> git checkout master
>> git merge my_branch
>>
>> All good. Then what? Since I can't do just 'git push', I try git-review again. The same thing happens. I have to run through the whole interactive rebase routine and resolve each of my commits from my_branch manually. I do that, then run 'git-review' again. Now I get this error message:
>>
>> remote: Hint: A potential Change-Id was found, but it was not in the footer of the commit message.
>> To ssh://otto [at] gerrit:29418/analytics/reportcard.git
>> ! [remote rejected] HEAD -> refs/for/master/master (missing Change-Id in commit message)
>> error: failed to push some refs to 'ssh://otto [at] gerrit:29418/analytics/reportcard.git'
>>
>> Each of the commits I merged from my_branch come with their own Change-Id in the commit messages. But these commits are now merge commits (I think?), so they have information about the merge and any conflicts in the commit message below the original Change-Id. I think this is confusing Gerrit, because it doesn't see the Change-Id in the footer.
>>
> There is a bug in git that causes merge commits to not automatically
> get Change-IDs. After generating a merge commit, you need to run git
> commit --amend , then save without changing anything. That makes sure
> the commit-msg hook is run and the Change-ID is appended.
>
> Roan
>
> _______________________________________________
> Wikitech-l mailing list
> Wikitech-l [at] lists
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l


_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


otto at wikimedia

Feb 21, 2012, 9:44 AM

Post #4 of 11 (1353 views)
Permalink
Re: Git + Gerrit is a toughy [In reply to]

> There is a bug in git that causes merge commits to not automatically
> get Change-IDs. After generating a merge commit, you need to run git
> commit --amend , then save without changing anything. That makes sure
> the commit-msg hook is run and the Change-ID is appended.
Yeah, I tried this, but no luck :( waaaaaah. I'm googling and trying other things to commit, but I'm still a little lost.

Also, any idea why git-review would say this every time I try to commit?

[~/Projects/wm/analytics/reportcard] (master)[29c6b47]$ git-review
You have more than one commit that you are about to submit.
The outstanding commits are:

29c6b47 (HEAD, master) observation.py - comments
14a771a test commit for git branch push
73dd606 Buncha mini changes + hackiness to parse a few things. This really needs more work
2d37c13 pipeline/user_agent.py - adding comment that this file should not be used
5892eb8 Adding loader.py - first hacky loader, just so we can get some data into mysql to work with.
e3fb30b Renaming the concept of variables to 'traits'. Allowing trait_sets to be specified so that we don't record HUGE amounts of data.
d0de74b base.py - adding schema in comments. Got lots of work to do to make this prettier
328e55d Trying my darndest to clean things up here! I've cloned a new repo, and am checking in my non-committed (an non-approved?) changes into this new branch. Hopefully gerrit will be happier with me.


This smells of me doing something really wrong.

Thanks!
-Ao


On Feb 18, 2012, at 2:31 AM, Roan Kattouw wrote:

> On Sat, Feb 18, 2012 at 2:47 AM, Andrew Otto <otto [at] wikimedia> wrote:
>> 2. Do I need to rebase every time I push for review?
>>
>> I don't quite understand what is going on here. I've installed git-review and am using this to push to git. It does a rebase by default. I'm not sure if I should be turning that off or not. Rebases seem like a bad idea unless you really need to do them. I think git-review is doing a rebase by default so it can squash all of your local commits into one big review commit before pushing. Yuck! This would surely mean fewer commits to review in Gerrit, but it destroys real the history. It is making git work more like subversion, where you just work locally until everything is good and then have one big commit. I should be able to commit often and be able to share my commits with other developers before having everything reviewed.
>>
> Yes, you need to rebase before you push. The rebase does not exist to
> squash multiple commits into one, but to ensure that your commit can
> be merged cleanly. This fits the gated trunk model, but it looks like
> you don't necessarily want to gate your working branch at all, just
> your master. IMO you should ask Ryan to set up direct push access for
> your working branches, so you can just git push into them directly,
> bypassing review. You can then merge your branch into master, and
> submit that merge commit for review.
>
>
>> 3. How does Gerrit handle merges? Do all merge commits need to be re-approved?
>>
> Yes.
>
>> 4. What should I do in the following situation?
>>
>> I have a branch I recently made from master. I've made some changes and pushed them to gerrit. My changes have been approved. Now I want to sync master into my branch. I do
>>
>> git merge master
>>
> Why did you merge master into your branch, rather than merging your
> branch into master? That doesn't make much sense to me.
>
>> Then resolve any conflicts and commit. How should I push these changes? The commits that make up the merge have already been approved in gerrit on the master branch. Do I need to push for review using git-review? They've already been approved, so I would think not. But gerrit will currently not allow me to push without using git-review (is that because the commits need a Change-Id?).
>>
> Yes, you need to submit the merge commit for review. If some commits
> don't have a Change-Id, git-review can't submit them, but I don't see
> how that could be the case. You said the commits were already approved
> in gerrit, *and* they don't have a Change-Id? Those things can't both
> be true.
>
>> Since gerrit doesn't let me do a regular git push to push my master merge to the remote branch I am tracking, I do git-review.
> Perhaps you should ask for regular pushes to be allowed if you're not
> using the review workflow for that branch, see also above.
>
>> This does rebase by default, so for some reason I am stuck having to resolve every single commit that was made to master in order to get the merge to push. This takes quite a while, but I did it, and once the interactive rebase was finished I was able to git-review to push the merge from master.
>>
>> Great. Now I that my branch is in sync with master again, I want to merge it into master.
>>
>> git checkout master
>> git merge my_branch
>>
>> All good. Then what? Since I can't do just 'git push', I try git-review again. The same thing happens. I have to run through the whole interactive rebase routine and resolve each of my commits from my_branch manually. I do that, then run 'git-review' again. Now I get this error message:
>>
>> remote: Hint: A potential Change-Id was found, but it was not in the footer of the commit message.
>> To ssh://otto [at] gerrit:29418/analytics/reportcard.git
>> ! [remote rejected] HEAD -> refs/for/master/master (missing Change-Id in commit message)
>> error: failed to push some refs to 'ssh://otto [at] gerrit:29418/analytics/reportcard.git'
>>
>> Each of the commits I merged from my_branch come with their own Change-Id in the commit messages. But these commits are now merge commits (I think?), so they have information about the merge and any conflicts in the commit message below the original Change-Id. I think this is confusing Gerrit, because it doesn't see the Change-Id in the footer.
>>
> There is a bug in git that causes merge commits to not automatically
> get Change-IDs. After generating a merge commit, you need to run git
> commit --amend , then save without changing anything. That makes sure
> the commit-msg hook is run and the Change-ID is appended.
>
> Roan
>
> _______________________________________________
> Wikitech-l mailing list
> Wikitech-l [at] lists
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l


_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


hashar+wmf at free

Feb 23, 2012, 1:43 PM

Post #5 of 11 (1235 views)
Permalink
Re: Git + Gerrit is a toughy [In reply to]

Le 21/02/12 18:44, Andrew Otto a écrit :
> [~/Projects/wm/analytics/reportcard] (master)[29c6b47]$ git-review
> You have more than one commit that you are about to submit.
> The outstanding commits are:
>
> 29c6b47 (HEAD, master) observation.py - comments
> 14a771a test commit for git branch push
> 73dd606 Buncha mini changes + hackiness to parse a few things. This really needs more work
> 2d37c13 pipeline/user_agent.py - adding comment that this file should not be used
> 5892eb8 Adding loader.py - first hacky loader, just so we can get some data into mysql to work with.
> e3fb30b Renaming the concept of variables to 'traits'. Allowing trait_sets to be specified so that we don't record HUGE amounts of data.
> d0de74b base.py - adding schema in comments. Got lots of work to do to make this prettier
> 328e55d Trying my darndest to clean things up here! I've cloned a new repo, and am checking in my non-committed (an non-approved?) changes into this new branch. Hopefully gerrit will be happier with me.
>
> This smells of me doing something really wrong.

It seems to be a git-review safe guard to prevents someone from sending
several commits. Each of them would make Gerrit generates several
changes. To those wondering why one would have multiple commits, three
use cases come to mind.

I will give you the solution at the end of this post.


1) high frequency tradin^H^H^H^H^H^H committing

Some people, me for example, do local commits very often, then squashes
them before submitting the final patch. Git squashing means regrouping
multiples commits in just one. Imagine I have made 4 commits locally,
possibly using my mother tongue (french), in such case, using git-review
will have me submitting a commit list like:

abcde1 oh mygod
d30909 variable fun
f39004 je ne sais plus ce que c'est
439090 before lunch

f39004 is some French meaning "I don't remember what was that" which
does not describe the commit change (hint: using French will be
perfectly valid once we start migrating from English).

Anyway, git-review would generates 4 changes out of the 4 commits above.
Not that helpful is it?

Instead I would have wanted to regroup them and write a nice commit
description. For example:

30490 (bug 1234) fix issue in feature foobar


2) newbie spamming gerrit

This happen when you first play with Gerrit.

In subversion world, whenever you submit a new patch (svn commit) it is
going to be written down in the central repository. You will not be able
to change it, hence any subsequent submission based on it are guaranteed
it is not going to change.

In git world, as I understand it, each commit as a parent commit. The
reference is a sha1 based on the content of the commit. Whenever you
change a commit, every children, grand-children .. will have their sha1
recomputed.

Enter in Gerrit world, when we send a commit in a queue, there is no
guarantee that commit will end up in the reference repo. It might be
amended or simply rejected. So your list of commit will be recomputed
and all child / grand children will need to be resubmitted.

Guess that? That will update of all those Gerrit changes, making mass
email spam / jenkins rebuild etc.


3) mixing features

You could well be mixing two different changes. Maybe you have made a
commit to fix bug 1234 and two days later a fix for bug 6789. Those
should really be two different changes.



In conclusion, it is best to use a local branch for any bug / feature
you might be working on actually. Branches are cheap in git, they are
just a pointer. Once you are happy with your small branch, squash the
commits and submit the end result. Less changes, less spam.

If you really want to *spam* force git-review to do it with a yes card:

$ git-review --yes

But you probably want to use branch / squash instead.


:-)

--
Antoine "hashar" Musso
Migration to French is not scheduled yet


_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


roan.kattouw at gmail

Feb 24, 2012, 3:27 PM

Post #6 of 11 (1192 views)
Permalink
Re: Git + Gerrit is a toughy [In reply to]

On Thu, Feb 23, 2012 at 1:43 PM, Antoine Musso <hashar+wmf [at] free> wrote:
> 2) newbie spamming gerrit
>
> This happen when you first play with Gerrit.
>
> In subversion world, whenever you submit a new patch (svn commit) it is
> going to be written down in the central repository. You will not be able to
> change it, hence any subsequent submission based on it are guaranteed it is
> not going to change.
>
> In git world, as I understand it, each commit as a parent commit. The
> reference is a sha1 based on the content of the commit. Whenever you change
> a commit, every children, grand-children .. will have their sha1 recomputed.
>
> Enter in Gerrit world, when we send a commit in a queue, there is no
> guarantee that commit will end up in the reference repo. It might be amended
> or simply rejected. So your list of commit will be recomputed and all child
> / grand children will need to be resubmitted.
>
> Guess that? That will update of all those Gerrit changes, making mass email
> spam / jenkins rebuild etc.
>
You're right that this is the biggest problem with stacked commits: if
you have a dependency chain A-B-C and B is amended or abandoned, you
have to rebase C somehow. A lesser problem is that if A and C are
approved, but B hasn't been reviewed yet, A will be merged but C won't
be (because it can't be merged without also merging B, and B has not
been approved yet).

So yeah, we want to encourage people to use separate branches for
unrelated commits, so that B doesn't depend on A if A and B are
totally unrelated to each other. I've been trying to work that into
the various documentation pages, and Sumana let me put it in her git
introduction talk script too :)

Roan

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


innocentkiller at gmail

Feb 24, 2012, 3:46 PM

Post #7 of 11 (1189 views)
Permalink
Re: Git + Gerrit is a toughy [In reply to]

On Fri, Feb 24, 2012 at 6:48 PM, Platonides <Platonides [at] gmail> wrote:
> There's no way to treat a set of commits as a bundle?
> What happens if a developer wants to merge his extension on which he has
> been working (in Git) for months?
>
> I am assuming:
> * The extension will get a full review.
> * The author wants to keep the extension versioning (it could even be
> already published in eg. GihtHub).
>
> Will gerrit force it to spawn dozens of commit reviews?
>

For situations in which we want to pull in some git history without
spamming gerrit, we can do a once-off straight push before forcing
commits through review.

It's what we're doing for the initial import.

-Chad

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


Platonides at gmail

Feb 24, 2012, 3:48 PM

Post #8 of 11 (1191 views)
Permalink
Re: Git + Gerrit is a toughy [In reply to]

On 25/02/12 00:27, Roan Kattouw wrote:
> You're right that this is the biggest problem with stacked commits: if
> you have a dependency chain A-B-C and B is amended or abandoned, you
> have to rebase C somehow. A lesser problem is that if A and C are
> approved, but B hasn't been reviewed yet, A will be merged but C won't
> be (because it can't be merged without also merging B, and B has not
> been approved yet).
>
> So yeah, we want to encourage people to use separate branches for
> unrelated commits, so that B doesn't depend on A if A and B are
> totally unrelated to each other. I've been trying to work that into
> the various documentation pages, and Sumana let me put it in her git
> introduction talk script too :)
>
> Roan

There's no way to treat a set of commits as a bundle?
What happens if a developer wants to merge his extension on which he has
been working (in Git) for months?

I am assuming:
* The extension will get a full review.
* The author wants to keep the extension versioning (it could even be
already published in eg. GihtHub).

Will gerrit force it to spawn dozens of commit reviews?


_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


hashar+wmf at free

Feb 25, 2012, 1:20 AM

Post #9 of 11 (1195 views)
Permalink
Re: Git + Gerrit is a toughy [In reply to]

Le 25/02/12 00:48, Platonides a écrit :
> There's no way to treat a set of commits as a bundle?

Not really. Each commit is considered by Gerrit as a new change. If you
have a bundle of commits, you either:

1) squash them in a single commit, losing all history but generating
only one change.

2) submit all commits, which makes as many changes request which can
then each be reviewed.

In the second form, whenever a commit is amended and resubmitted, every
descendant will be resubmitted as well since the sha1 is changed :-)


> What happens if a developer wants to merge his extension on which he has
> been working (in Git) for months?

As Roan said, in such a case we will probably want to bypass Gerrit. We
could review the various commits before manually merging/pulling all
those changes directly in the repo.

Some people will be allowed to bypass Gerrit entirely just so they can
handle that kind of situations.

--
Antoine "hashar" Musso



_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


roan.kattouw at gmail

Feb 25, 2012, 4:04 PM

Post #10 of 11 (1183 views)
Permalink
Re: Git + Gerrit is a toughy [In reply to]

On Sat, Feb 25, 2012 at 1:20 AM, Antoine Musso <hashar+wmf [at] free> wrote:
> Le 25/02/12 00:48, Platonides a écrit :
>
>> There's no way to treat a set of commits as a bundle?
>
>
> Not really. Each commit is considered by Gerrit as a new change. If you have
> a bundle of commits, you either:
>
>  1) squash them in a single commit, losing all history but generating only
> one change.
>
>  2) submit all commits, which makes as many changes request which can then
> each be reviewed.
>
I believe there is an option number 3, which is to make your changes
in a branch (you should be doing this anyway), then merge that branch
into master with the --no-ff option (this ensures a merge commit is
created even if git thinks it's not strictly necessary) and submit the
merge commit for review. To be fair I've never actually tried this in
cases where the commits in the branch weren't already in Gerrit (I've
submitted merge commits for review before, but only to merge in
branches that were already managed by Gerrit), so I'm not 100% sure it
will be handled the way I think it will be.

Roan

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


otto at wikimedia

Feb 27, 2012, 4:01 PM

Post #11 of 11 (1369 views)
Permalink
Re: Git + Gerrit is a toughy [In reply to]

> In conclusion, it is best to use a local branch for any bug / feature you might be working on actually. Branches are cheap in git, they are just a pointer. Once you are happy with your small branch, squash the commits and submit the end result. Less changes, less spam.
We've had a few discussions around the office about this, but I'll comment here too.

This works as long as you are not trying to collaborate with other people. There are 5 of us trying to use our analytics repository for development work. We are doing fake-scrum, and every day we talk about what has been worked on. We are using git/gerrit to share this work. Committing often to only local branches doesn't help us for collaboration.

Roan and Ryan have helped us work out a workflow that will work for us decently. We can push directly to the main repository when we need to, and use gerrit for all other times. Depending on the changes, we'll end up reviewing code when it is first pushed to a remote branch, OR later when it is merged into master. That way we only have to review code at a single point, rather than multiple. This requires that everyone our in our team is communicating and responsibly pushing for review. Our team is small so that's no problem for us. But this will be a bigger problem for MediaWiki fo sho.

Thanks Antoine!

- otto


http://ottomata.com
http://www.flickr.com/photos/OttomatonA
http://www.couchsurfing.org/people/otto



On Feb 23, 2012, at 1:43 PM, Antoine Musso wrote:

> Le 21/02/12 18:44, Andrew Otto a écrit :
>> [~/Projects/wm/analytics/reportcard] (master)[29c6b47]$ git-review
>> You have more than one commit that you are about to submit.
>> The outstanding commits are:
>>
>> 29c6b47 (HEAD, master) observation.py - comments
>> 14a771a test commit for git branch push
>> 73dd606 Buncha mini changes + hackiness to parse a few things. This really needs more work
>> 2d37c13 pipeline/user_agent.py - adding comment that this file should not be used
>> 5892eb8 Adding loader.py - first hacky loader, just so we can get some data into mysql to work with.
>> e3fb30b Renaming the concept of variables to 'traits'. Allowing trait_sets to be specified so that we don't record HUGE amounts of data.
>> d0de74b base.py - adding schema in comments. Got lots of work to do to make this prettier
>> 328e55d Trying my darndest to clean things up here! I've cloned a new repo, and am checking in my non-committed (an non-approved?) changes into this new branch. Hopefully gerrit will be happier with me.
>>
>> This smells of me doing something really wrong.
>
> It seems to be a git-review safe guard to prevents someone from sending several commits. Each of them would make Gerrit generates several changes. To those wondering why one would have multiple commits, three use cases come to mind.
>
> I will give you the solution at the end of this post.
>
>
> 1) high frequency tradin^H^H^H^H^H^H committing
>
> Some people, me for example, do local commits very often, then squashes them before submitting the final patch. Git squashing means regrouping multiples commits in just one. Imagine I have made 4 commits locally, possibly using my mother tongue (french), in such case, using git-review will have me submitting a commit list like:
>
> abcde1 oh mygod
> d30909 variable fun
> f39004 je ne sais plus ce que c'est
> 439090 before lunch
>
> f39004 is some French meaning "I don't remember what was that" which does not describe the commit change (hint: using French will be perfectly valid once we start migrating from English).
>
> Anyway, git-review would generates 4 changes out of the 4 commits above. Not that helpful is it?
>
> Instead I would have wanted to regroup them and write a nice commit description. For example:
>
> 30490 (bug 1234) fix issue in feature foobar
>
>
> 2) newbie spamming gerrit
>
> This happen when you first play with Gerrit.
>
> In subversion world, whenever you submit a new patch (svn commit) it is going to be written down in the central repository. You will not be able to change it, hence any subsequent submission based on it are guaranteed it is not going to change.
>
> In git world, as I understand it, each commit as a parent commit. The reference is a sha1 based on the content of the commit. Whenever you change a commit, every children, grand-children .. will have their sha1 recomputed.
>
> Enter in Gerrit world, when we send a commit in a queue, there is no guarantee that commit will end up in the reference repo. It might be amended or simply rejected. So your list of commit will be recomputed and all child / grand children will need to be resubmitted.
>
> Guess that? That will update of all those Gerrit changes, making mass email spam / jenkins rebuild etc.
>
>
> 3) mixing features
>
> You could well be mixing two different changes. Maybe you have made a commit to fix bug 1234 and two days later a fix for bug 6789. Those should really be two different changes.
>
>
>
> In conclusion, it is best to use a local branch for any bug / feature you might be working on actually. Branches are cheap in git, they are just a pointer. Once you are happy with your small branch, squash the commits and submit the end result. Less changes, less spam.
>
> If you really want to *spam* force git-review to do it with a yes card:
>
> $ git-review --yes
>
> But you probably want to use branch / squash instead.
>
>
> :-)
>
> --
> Antoine "hashar" Musso
> Migration to French is not scheduled yet
>
>
> _______________________________________________
> Wikitech-l mailing list
> Wikitech-l [at] lists
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l


_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Wikipedia wikitech RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.