couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dale Harvey <d...@arandomurl.com>
Subject Re: we need more reviews
Date Mon, 20 Jan 2014 12:26:55 GMT
I fully agree, its something I mentioned at the couchdb conf in vancouver,
a review first system encourages contributions and has multiple benefits

 * At least 2 people look at the code, less likely to push silly mistakes
 * Can codify and practice review rules
 * Its much easier to view the current activity in the project
 * Can bring up points of discussion before its too late

But I think the most important thing is that it removes the burden of
responsibility from the committer to the project as a whole, also hugely
beneficial is that it makes it particularly obvious when a patch has reach
a stalemate and forces someone to make the call.

For reference on PouchDB every committer is trusted to push code, nobody
(including myself) pushes their own code to master, it goes in the PR queue
and gets a +1 from any other committer (who will usually push it), thats
essentially the same process we use at mozilla and coming to think of it
any other project I have worked on, any commiter has the ability to -1 a
patch at which point they give a reason and usually some solution gets
agreed on




On 20 January 2014 12:07, Benoit Chesneau <bchesneau@gmail.com> wrote:

> Hi All,
>
> Over the last few months, we have had a pretty good track in commits --
> which is great! However I think we can do better working as a team on
> this. I am sure others also feel the same way. For example:
>
> - Some code is still landing in master with little or absence of review
>   in JIRA nor the mailing list
>
> - There is insufficient discussion, sometimes a short word on IRC or out
>   of sight of the mailing list. Some of the discussions feel more like a
> monologue, or don't reach a satisfactory conclusion or clear decision.
> For example, the last discussion about metadata never reached a
> conclusion and as a result, no changes have been made.
>
> I really want to find a way to fix that. It is our responsibility to fix
> that.
> The project should count as a real team work.
>
> A team is not a collection of individuals working on their side
> where`git commit` or `jira close` are the only meeting points. That also
> opens the doors to more conflicts, since people have to react against a
> thing that have been done. We should discuss and review more the code
> (and a review should be answered). Actually I do think that no code
> should land in master without at least one or more (depending how
> sensitive the patch is) public review. But we may be less strict. At
> least nothing should come without a context. It’s also true for the
> commits messages. There may be a discussion on IRC that explain that but
> IRC is not a reliable source. People should understand the why and the
> what for without having to read the code and should understand the
> direction and eventually discuss it before it is taken.
>
> All of these only require some guidelines. But we should start to
> enforce them and make them public.  Developers should act together
> again.
>
> In term of action I think we could:
>
> - have review system with votes on the patch before inclusion in the
>   master  (as in the apache https project). Many projects are working
> like this and this is part of their success. We should of course explain
> why a review is important and make sure that reviews are not transformed
> in a fight between some egos.
> - have some guidelines on how to discuss features
> - make it clear that IRC is not the place where decisions are took and
>   enforce the ML usage
> - make sure that people are writing good commit messages. Some projects
>   (erlang. linux are coming to my mind) make sure to fix them before
> they land in maint or master branches.
>
> Thoughts?
>
>
> - benoit
>
> NB: this a repost on public @dev mailing-list from the PMC mailing-list.
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message