Return-Path: X-Original-To: apmail-ignite-dev-archive@minotaur.apache.org Delivered-To: apmail-ignite-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 104191050D for ; Wed, 24 Dec 2014 16:45:16 +0000 (UTC) Received: (qmail 15434 invoked by uid 500); 24 Dec 2014 16:45:15 -0000 Delivered-To: apmail-ignite-dev-archive@ignite.apache.org Received: (qmail 15404 invoked by uid 500); 24 Dec 2014 16:45:15 -0000 Mailing-List: contact dev-help@ignite.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@ignite.incubator.apache.org Delivered-To: mailing list dev@ignite.incubator.apache.org Received: (qmail 13997 invoked by uid 99); 24 Dec 2014 16:45:06 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 24 Dec 2014 16:45:06 +0000 X-ASF-Spam-Status: No, hits=1.5 required=5.0 tests=HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of dsetrakyan@gridgain.com designates 74.125.82.48 as permitted sender) Received: from [74.125.82.48] (HELO mail-wg0-f48.google.com) (74.125.82.48) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 24 Dec 2014 16:45:01 +0000 Received: by mail-wg0-f48.google.com with SMTP id y19so11589421wgg.21 for ; Wed, 24 Dec 2014 08:42:25 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:content-type; bh=gpbjRByuf1cVu2M1tVQu7YbS2OtFzyreRNNx9frqqo0=; b=PhuAZenRY12ycRbYz5QQRRt+ef+1k60BlD/Xn4MG6x191YaCygvSQ4uP1d68oxg4fv GGKAu2aoku+/pC28z8VObvbOHG44kCCdCM1nEZ+Lykvk6suj+4ptU9Da1qw86NPH5mHC Z5oWJsxSxAZPjn2I54eS6kY3OXpRm3oibfKyIOJcYqEegYh4DE4dU4oMCsdJP3Zen0d2 v23koGxGNBRl/s6qlI+oNpNRjDiok0zvAj0+rpd8IC3HZ3+E+4MM7LzO3OCruJp5+trE I6IQ8dBfqefA8SO/i6Wf1mgdavemQ3KQyy6YGvJdcMMMMCL71OuaW2LWJJC6VyeZo3fa 2USg== X-Gm-Message-State: ALoCoQlg0TP+voQO0GcxffCKvoUhWA9KuwcFLcHBb7Tm0C3+gOMzFC1TOr6oFAePZeB57Wx/1Bll X-Received: by 10.194.237.101 with SMTP id vb5mr64983937wjc.30.1419439345003; Wed, 24 Dec 2014 08:42:25 -0800 (PST) MIME-Version: 1.0 Received: by 10.194.51.97 with HTTP; Wed, 24 Dec 2014 08:41:44 -0800 (PST) In-Reply-To: References: <20141223015407.GT1655@boudnik.org> <549ABEA8.7050005@apache.org> From: Dmitriy Setrakyan Date: Wed, 24 Dec 2014 08:41:44 -0800 Message-ID: Subject: Re: Patch review process: is there any? To: dev@ignite.incubator.apache.org Content-Type: multipart/alternative; boundary=089e013d2028f0863c050af8f9c6 X-Virus-Checked: Checked by ClamAV on apache.org --089e013d2028f0863c050af8f9c6 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable By the way, I am not sure if the process I suggested qualifies as CTR or RTC. Essentially, you commit to your own branch before the review, but you are allowed to commit to the main release branch only after review. D. On Wed, Dec 24, 2014 at 8:37 AM, Dmitriy Setrakyan wrote: > I would prefer that we establish some rules before committing to the main > release branch. > > 1. For every release we create a main release branch. > 2. Every ticket must be worked on in a separate GIT branch. For > example, if the ticket name is IGNITE-xxx, then the branch name should= be > ignite-xxx. > 3. Prior to merging to the main release branch, all CI tests in > ignite-xxx branch must pass (we still need to work out how CI will wor= k for > the Ignite project) > 4. Every branch must pass at least 1 peer review from another > committer prior to its merge to the main release branch. > > Such process guarantees that the main release branch always passes all CI > tests, and the code has been reviewed by more than 1 person. > > Is there a standard place for Apache projects where such process is > documented? > > D. > > On Wed, Dec 24, 2014 at 5:24 AM, Branko =C4=8Cibej wro= te: > >> On 23.12.2014 02:54, Konstantin Boudnik wrote: >> > Guys, >> > >> > first of: congrats on moving forward with code base import and fully >> > functional website! I also see that people are working quite hard on >> adding >> > new stuff and fixing some post-import issues. Which leads me to this >> generic >> > question: >> > - what's the formal review process of changes, if any? >> > >> > From what I see in Apache - and I'm begging veterans here to correct m= e >> - >> > there are two different school of thoughts: >> > # Review Then Commit (or RTC) >> > # Commit Then Review (or CTR) >> > >> > There's no right or wrong way of doing this. And perhaps you might wan= t >> to >> > employ a practice of CaTHWI (or Commit and The Hell With It). It's >> really up >> > to you. >> >> There is one huge advantage of CTR over RTC within the context of the >> ASF: RTC tends to create a more closed, less meritocratic community, and >> also tends to slow down development of new features. More than once, >> I've seen projects that use RTC fragment into a mess of petty >> dictatorships, with each significant module being lorded over by a >> single developer who had the effective (if not formal) total control >> over the direction of development. This situation is as murderous to an >> open-source community as a single-point-of-failure is to a distributed >> application ... in fact, the similarities are amazingly close. >> >> > Which one is/will be a common practice in the Ignite (incubating)? I >> haven't >> > seen much of discussion on the topic, so I presume you keep on using >> the same >> > practice as it was accepted in the engineering team behind of the >> product. But >> > as the community develops and grows it might be a good idea to think >> about >> > some formal steps in this direction. >> >> I'd also like to bring up another related topic: the structure of the >> community. The ASF effectively recognises three categories of community >> members: >> >> * Committer >> Committers have the right to commit changes to any part of the >> project repository. In the CTR model, they're assumed to be >> competent to decide whether a change needs prior peer review or >> whether to just commit it and expect review after the fact; the >> latter being the most common case. >> * PMC >> Members of the PMC are committers who also vote on release >> artefacts, and they decide about accepting new committers to the >> project, and promoting committers to PMC membership. >> * Contributors >> These are members of the project community at large who do not have >> commit privileges, but contribute to the project in many different >> ways; perhaps by sending patches, or writing documentation, or >> helping out on the users@ list, etc. Future committers typically >> come from this group. >> >> At the Subversion project, we have a fourth category: >> >> * Partial Committer >> Has commit privileges like any other committer, but with the >> understanding that they're only free to practice CTR within a >> well-defined subset of the code (e.g., language translators would >> have partial commit access to a translation; a GSoC student would >> have partial commit access to a branch on which they're developing >> their project code). Otherwise, they're expected to follow RTC. We >> do not enforce this by any kind of access control; any member of the >> community is expected to follow the rules that they adopted when >> they accepted commit privileges. >> >> Some ASF projects (APR and Subversion are examples) do not make a >> difference between a committer and a PMC member, so for example at >> Subversion, we only have PMC, partial committers and everyone else. The >> argument for committer=3D=3DPMC is a simpel one: if you trust somebody >> enough to mess with your code, you should also trust them with oversight >> of the project, and the other way around. >> >> Some projects (Subversion again being one of them) give any ASF member >> commit privileges to the code without going through a formal vote. The >> assumption being that an ASF member will understand that they cannot >> make major changes without discussing them with the community, but it's >> fine to commit obvious bug fixes without going through the process of >> patch submission and review. >> >> -- Brane >> >> > --089e013d2028f0863c050af8f9c6--