Daniel Friesen write:
I want whatever review system we use to support real review branches;
- Review sets are heads with multiple commits, not single commits.
- -no-ff merge commits are used so that only the final commit code makes
it directly into master.
We have disallowed branch creation and merging for now, that can definitely be opened up. Chad recently opened the sandbox reference and we have a few of them:
remotes/gerrit/sandbox/apramana/gsoc remotes/gerrit/sandbox/demon/foo-bar remotes/gerrit/sandbox/devunt/oauth remotes/gerrit/sandbox/ori/hacks
They are actual branch we can most definitely merge -no-ff back in master. http://www.mediawiki.org/wiki/Gerrit/personal_sandbox
- We don't amend commits, we make new ones to the head of the review
branch.
You will be able to do that. I am afraid the end result is not going to look nice history wise with ton of followups to fix previous commits. That will definitely make finding the root cause of bug and its context harder to find out. Think about a commit that introduce a bug for which the commit message is:
Ahh typo in 3124a09 fix it
It is not going to be helpful :-D
Scrap all the other complaints. This pattern of amending commits is by far the worst thing about our current workflow.
I firmly disagree with you there. It let someone send some code then enhance it without cluttering everyone else code. That let you work in a collaborative way, finding out what the other person has done patch after patch. We eventually end up with a patchset that only got published (merged) to everyone whenever it is close to perfect. That makes our code history nicer and push the review stress to the submitters (lot of people) instead of toward the reviewers (few people).
I do agree though that the system can be confusing and is poorly toolized. git-review is a step to abstract the whole patchset system, but I am sure we can make it a better/easier tool to use.
Development history and minor contributions that never make it into the repo.
I think we do not care about the history of a change. The typo / comment update / logic errors, I am sure we do not care about. If someone has a change that requires to be split in several changes, that should easier be a branch (see sandbox above) or a chain of changes made interdependants (which is really just a local branch).
How would you do that? Well lets say I want to update our documentation to several area. I would first create a local branch:
git checkout -b doc -t origin/master <do my doc updates, commit as needed>
I then push that local branch to Gerrit (git push doc:refs/for/master), that craft a change per commit. Whenever one of the commit need to be amended I edit my local branch commits with git rebase --interactive. Once happy I repush the whole branch and Gerrit magically update all the patchsets.
The drawback is that editing just a commit out of several one will still trigger a new patchset to all the child commits although there is no code change per see (the parent got changed). That could be detected in Gerrit I guess. We might have Gerrit to automatically rebase children whenever a parent is modified. Gerrit could also use a feature to prevent the chain of commits to be submitted until they have all been reviewed. Once the last change is submitted, that will trigger a git merge --no-ff. We currently have to handle that manually by submitting the first commit last.
"Committer" going to the person who fixes a typo rather than the person who writes the code.
The Author field is kept around (unless someone amend it). See as an example https://gerrit.wikimedia.org/r/#/c/5550/, I am the patch author, Saper sent patchset 3 but I am still mentioned as the author.
git does not support multiple authors as far as I know, but one could add himself in the commit message by adding a new field such as:
Co-authored-by: John Doh johndoe@example.org
Confusion over whether someone has rebased or let unrelated changes leak into their patchset.
That is more a matter of educating our developers. Whenever I submit a new patchset, I add a cover message in Gerrit to introduce what that change did even if it is "just a rebase".
Huge development effort by multiple people potentially turning into a single commit with a single committer. ...
As I said previously, huge effort involving multiple persons could be made branch.
At this point I think the real question we have is this: "Can we reasonably get Gerrit to support real review branches?"
The answer is most definitely yes. We would need to open up branches creations to everyone.
If that answer to that question is no. Or "not without very significant development". Then the only option we have is to track down the system that we can get to support that with the most reasonable amount of development effort.
I tend to agree on that :-)