On 1/5/12 9:06 AM, Jonathan Ellis wrote:
On Thu, Jan 5, 2012 at 10:58 AM, Sylvain Lebresne<sylv...@datastax.com> wrote:
Again, I was more talking about the only reasonable solution I saw.
Because to be clear, if the history for some issue 666 in say trunk looks like:
commit eeee: last nits from reviewer
commit dddd: oops, typo that prevented commit
commit cccc: some more fix found during review
commit bbbb: refactor half of preceding patch following reviewer comments
commit aaaa: Do something awesome - patch for #666
Don't forget
commit ffff:<merge> (i.e., resolve conflicts introduced in master post-branch)
So basically my question is how do we meld all those commits that will
necessarily happen due to the nature of distributed reviews so that our
main history don't look like shit? And if the answer is "we don't" then
I'm not too fond of that solution.
+1
One thing to consider is something like gerrit which allow code reviews
before you submit to "trunk".
Either way if you work on a branch separate branch form "trunk" you can
publish changes for people to review.
If they have a nit. You fix it *in the same commit* using git commit
--amend.
Step # 1: Issue fixer
git checkout -b issue666 origin/trunk
*hack hack*
git commit -a
Send e-mail to reviewers telling them to do pull.
Step #2: Reviewer:
git checkout -b issue666
git pull <person submitting code's git url> issue666
* review review*
* Send e-mail to issue fixer pointing out stuff*
Step #3: Issue fixer
*fix pointed out stuff*
git commit --amend
*Send out email "I've fixed it please review again"
Step #4: Reviewer
git checkout issue666 # assuming they have been working on something else
git reset --hard HEAD^1 # this will get you back to the state before you
pulled issue666
git pull <person submitting code's git url> issue666
* review review*
* Send e-mail to issue fixer pointing out stuff*
You can do steps #3 and #4 as many times as necessary.
Once everyone is happy, either one of the reviewers or the issue fixer can:
git checkout trunk
git rebase issue666
Also if trunk moves along during the review process the issue creator
can just:
git checkout issue666
git rebase trunk
and then send things for review again.
Gerrit makes a lot of the "send e-mail to pull" and "send e-mail
pointing stuff out" a lot easier.
HTH,
tedo
