[haiku] Re: How to improve Haiku

  • From: pulkomandy <pulkomandy@xxxxxxxxx>
  • To: haiku@xxxxxxxxxxxxx
  • Date: Sun, 27 Apr 2014 17:51:10 +0200

On Sun, Apr 27, 2014 at 02:42:40PM +0200, Pawel Dziepak wrote:
> Such approach, in my opinion, is much
> better than reviewing patches on trac, which I find very inconvenient
> for such purpose as it is not possible to add comments to an
> individual line or a part of the code. Also, I am not a big fan of
> github pull requests and I prefer the simplicity of mailing list and
> basic git tools, but I will be happy if any solution that improves
> code quality by mandatory code review will be adopted. Last, but not
> least, there is actually a reason why I didn't proposed such changes
> to our workflow earlier. While this may work very well with project
> when the vast majority of developers is employed full-time I am not
> sure it will work for us with our limited resources.

May I, once again, suggest we setup Gerrit and use the git workflow that
goes with it? Patches are submitted using a simple git push operation.
They enter the Gerrit code review system where line-by-line reviews and
votes (+1/-1) are possible. Gerrit can even be configured to enforce
patches to get a score of at least "+2" and no "-" from reviewers before
it can be applied. It seems the patches are properly rebased when merged
to the main tree.

The original patch author can make modifications to the patch, and
re-submit it, if done properly, this is appended to the existing Gerrit
entry so it's easy to review the new patch in context.

Gerrit also handles dependencies between patches ("this patch needs xxx
to be merged first").

I think this would lead to a much more solid state for the trunk, making
it much easier to get a version of it tagged "release", with a MUCH
shorter release cycle than we currently have, and more stable nightlies.

With such a great tool in place, reviewing patches will become a much
easier and rewarding job, and we can finally reply to people fast enough
so we have a chance of them fixing the issues. Getting a review after
two weeks that says "sorry, this patch you probably already gave up on
getting applied has some coding style violations..." isn't a good way to
get more contributors.

With a tool like that, I think it's possible to make code reviews
systematic, at least for "potentially breaking" changes.

I'll add that Gerrit also supports checking that the patch doesn't break
the build, and could easily be combined with an automated coding style
checker.

-- 
Adrien / PulkoMandy.

Other related posts: