[rules-dev] Peer reviewing on github with "commit comments": very useful

[Esteban Aliverti]

This is a great feature! Unfortunately, I had to learn it the hard way ;)

Best Regards,

XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

Esteban Aliverti
- Developer @ http://www.plugtree.com
- Blog @ http://ilesteban.wordpress.com


On Fri, Jan 28, 2011 at 5:58 AM, Geoffrey De Smet
<ge0ffrey.spam at gmail.com>wrote:

> Hi guys,
>
> Until now, when a code change was committed and pushed by someone else
> and you did not agree with the code change(s),
> there were only 3 things you could do:
> - Fix it and commit it, if you're a 90%+ sure that it's a mistake (and
> probably mail the original committer so he has a chance to disagree)
> - Postpone it and hope you run into the original committer on IRC (or
> mail him), still remember the issue, still remember the class and line
> name and discuss it...
> - Ignore it, especially if you're less than 50% sure that it's a problem.
> Most of the times, we probably ignored it, to avoid stepping on people
> toes and because it's a lot of work copying the affected code.
>
> But thanks to github, there's an 4th way: look up the commit on
> github.com and click left of the line(s) with which you don't agree.
> It has a couple of advantages:
> - Your concerns are hooked to the code line of your concerns, making it
> much easier for the original author to understand your point
> - The original author might invalidate your concerns or find a bug in
> your proposed changes
> - If the original author validates your concerns, he has learned and
> won't make the same mistake again
> - The original author might have made a similar mistake in other code
> (which only he knows about)
>
> Here's a recent successful conversation:
>
>
> https://github.com/droolsjbpm/droolsjbpm/commit/e9cbcf0e874cc2103ebda44ceafebed2473a556c#L0R359
>
> You can find a list of all commit comments here:
>   https://github.com/droolsjbpm/droolsjbpm/comments
>
> --
> With kind regards,
> Geoffrey De Smet
>
>
> _______________________________________________
> rules-dev mailing list
> rules-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/rules-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/rules-dev/attachments/20110128/dd6f5328/attachment.html