-1

I have a local code-review process wherein a reviewer adds an empty commit on top of a branch that its been reviewed and send s back to developer.

The developer then pushes it to canonical.

I want to write a pre-push hook which will see if last commit says that the code is reviewed and then append a "PEER-REVIEWED" word to the commit message of all commits on the branch and then push it to Canonical!

(Use: I can see any commit in my canonical and see if it is reviewed or not. If its reviewed then it will have "PEER-REVIEWED" word in it.)

  1. How practical is the approach?
  2. How do I automatically append the word in commit messages of all the commits getting pushed.

Thanks!

Mudassir Razvi
  • 1,783
  • 12
  • 33
  • You should think twice about this; it sounds like a really bad idea. Do you realise that this "peer-review process" involves rewriting the whole branch (i.e. changing the SHAs of all its commits)? That's probably not what you want. – jub0bs Feb 25 '15 at 10:29
  • Yes! I perfectly understand the consequences. Once the developer pushes the reviewed branch that branch becomes useless. So we recommend him to create a new branch. As to rewriting the history, the branch is not pushed to canonical yet, so what difference will it make if I rewrite the history thus changing the SHA's. – Mudassir Razvi Feb 25 '15 at 10:46
  • Awful idea. The way you guys are going about it I'd actually suggest: add a "PEER_REVIEWED" to all commits out of the bat and only consider it NOT reviewed if there was no empty commit at the top. :-P So... commit hook is enough. – LAFK 4Monica_banAI_modStrike Apr 18 '16 at 19:06

1 Answers1

1

It is totally useless to mark every commit as "peer-reviewed" in a branch, first because it is not true as you wrote that the review happens on the top of the branch (HEAD). Interim commits can be work-in-progress (and not properly working).

git commit has an --allow-empty argument and its primary use is to trigger hook scripts. So I suggest if the peer-review happened, simply add a new - empty - commit to the branch (it will go to the top), give it the commit message "peer-reviewed" and create a hook that checks for this commit message.

karatedog
  • 2,508
  • 19
  • 29
  • Yes that option is available, but. I use rebase to put commits on canonical master and not merge. So If some branch was pushed which does not have review commit and the nex push branch has it, then i have no way to find weather that commit was reviewed or not. – Mudassir Razvi Feb 25 '15 at 11:12
  • 1
    That is why I don't like `rebase` because it is destructive and I would lose all history, not to mention the dates when some feature branch were merged into master. And this means that your Canonical and the development repository have totally different history, and I like being able to pull the entire production repo and continue development from there. – karatedog Feb 25 '15 at 12:00