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 <john...@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 :-)
