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@gerrit.wikimedia.org: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@gerrit.wikimedia.org: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
On Sat, Feb 18, 2012 at 2:47 AM, Andrew Otto otto@wikimedia.org wrote:
- 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.
- How does Gerrit handle merges? Do all merge commits need to be re-approved?
Yes.
- 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@gerrit.wikimedia.org: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@gerrit.wikimedia.org: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
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@wikimedia.org wrote:
- 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.
- How does Gerrit handle merges? Do all merge commits need to be re-approved?
Yes.
- 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@gerrit.wikimedia.org: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@gerrit.wikimedia.org: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@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
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@wikimedia.org wrote:
- 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.
- How does Gerrit handle merges? Do all merge commits need to be re-approved?
Yes.
- 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@gerrit.wikimedia.org: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@gerrit.wikimedia.org: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@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
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.
:-)
On Thu, Feb 23, 2012 at 1:43 PM, Antoine Musso hashar+wmf@free.fr wrote:
- 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
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?
On Fri, Feb 24, 2012 at 6:48 PM, Platonides Platonides@gmail.com 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
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.
On Sat, Feb 25, 2012 at 1:20 AM, Antoine Musso hashar+wmf@free.fr 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
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.
- 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
- 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.
- 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@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
wikitech-l@lists.wikimedia.org