couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andy Wenk <a...@nms.de>
Subject Re: we need more reviews
Date Mon, 20 Jan 2014 12:46:17 GMT
On 20 January 2014 13:26, Dale Harvey <dale@arandomurl.com> wrote:

> 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.
> >
>

awesome to start this discussion. It was just some hours ago, that Hank
Knight exactly asked these questions. I am +1 on defining guidelines to
describe the process. It would not be the biggest problem to install gerrit
(https://code.google.com/p/gerrit/) as a review system somewhere but I
would like to ask if it is also ok to use the PR queue like Dale wrote
above. I think the process is well known and learned. The only big
difference is that it is still possible to push to master without review
and PR's. But I don't see a problem to establish a review process with PR's
when we define easy guidelines.

A word to git commit messages. If you want to read really informative
commit messages as an example, juts look at the PostgreSQL project.
Especially Tom Lane is writing awesome messages. So I am +100 to
informative and meaningful commit messages.

Cheers

Andy
-- 
Andy Wenk
Hamburg - Germany
RockIt!

http://www.couchdb-buch.de
http://www.pg-praxisbuch.de

GPG fingerprint: C044 8322 9E12 1483 4FEC 9452 B65D 6BE3 9ED3 9588

https://people.apache.org/keys/committer/andywenk.asc

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