Return-Path: X-Original-To: apmail-cordova-dev-archive@www.apache.org Delivered-To: apmail-cordova-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id B50E2E223 for ; Tue, 22 Jan 2013 19:33:51 +0000 (UTC) Received: (qmail 40534 invoked by uid 500); 22 Jan 2013 19:33:51 -0000 Delivered-To: apmail-cordova-dev-archive@cordova.apache.org Received: (qmail 40511 invoked by uid 500); 22 Jan 2013 19:33:51 -0000 Mailing-List: contact dev-help@cordova.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cordova.apache.org Delivered-To: mailing list dev@cordova.apache.org Received: (qmail 40502 invoked by uid 99); 22 Jan 2013 19:33:51 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 22 Jan 2013 19:33:51 +0000 X-ASF-Spam-Status: No, hits=-0.7 required=5.0 tests=RCVD_IN_DNSWL_LOW,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: domain of fil@adobe.com designates 64.18.1.233 as permitted sender) Received: from [64.18.1.233] (HELO exprod6og118.obsmtp.com) (64.18.1.233) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 22 Jan 2013 19:33:44 +0000 Received: from outbound-smtp-2.corp.adobe.com ([193.104.215.16]) by exprod6ob118.postini.com ([64.18.5.12]) with SMTP ID DSNKUP7pgDFjKvr0O3TfonnSbg1LlNz0B0LI@postini.com; Tue, 22 Jan 2013 11:33:23 PST Received: from inner-relay-4.eur.adobe.com (inner-relay-4b [10.128.4.237]) by outbound-smtp-2.corp.adobe.com (8.12.10/8.12.10) with ESMTP id r0MJXJC9028990; Tue, 22 Jan 2013 11:33:19 -0800 (PST) Received: from nacas03.corp.adobe.com (nacas03.corp.adobe.com [10.8.189.121]) by inner-relay-4.eur.adobe.com (8.12.10/8.12.9) with ESMTP id r0MJXIXL011997; Tue, 22 Jan 2013 11:33:18 -0800 (PST) Received: from nambxv01a.corp.adobe.com ([10.8.189.95]) by nacas03.corp.adobe.com ([10.8.189.121]) with mapi; Tue, 22 Jan 2013 11:33:17 -0800 From: Filip Maj To: "dev@cordova.apache.org" , "alunny@gmail.com" Date: Tue, 22 Jan 2013 11:36:21 -0800 Subject: Re: Can we talk about large features before they arrive in master? Thread-Topic: Can we talk about large features before they arrive in master? Thread-Index: Ac3411PM6bA+stNuRS+FgPhHz/O4HQ== Message-ID: In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: user-agent: Microsoft-MacOutlook/14.2.5.121010 acceptlanguage: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Virus-Checked: Checked by ClamAV on apache.org Not a bad idea, noted On 1/22/13 10:57 AM, "Andrew Lunny" wrote: >Outsider observation/note: probably useful for step 4 to tie into the CI - >getting the pull request to notify Fil's CI project. > >On 22 January 2013 10:53, Andrew Grieve wrote: > >> Here's a draft for a new "Committing Your Own Changes:" section on >> http://wiki.apache.org/cordova/CommitterWorkflow >> >> Step 1: Mail the Mailing-list >> - This is required if: >> - Your change will add/remove/change any public Cordova APIs. >> - You suspect that your change has a chance of being controversial >> - You would like feedback before you begin. >> >> When possible, try to phrase things in the form of a proposal. If no one >> objects (within a workday or two), then consider yourself to have Lazy >> Consensus=20 >>. >> >> Step 2: Ensure there is a JIRA issue. >> - JIRA issues are used for both new features and for bugs. >> - The "Fix For" field is used for the purpose of Release Notes. >> - The issues are also used to track which commits / topic branches are >> related to them. >> >> Step 3: Create a topic branch >> - Using a public topic branch is necessary only when either: >> 1. you would like to collaborate on the feature >> 2. you would like feedback on your code before committing >> - For small bugfixes, public topic branches are not required. >> - Note: You should never rebase a public topic branch! >> >> Step 4: Ask for a code review >> - If you are using a public topic branch, then you should ask for a >>code >> review when you consider it to be complete. >> - For now, use a github pull request. Soon, use reviews.apache.org. >> - Email the ML so that anyone who is available can have a look at your >> code. If you have someone in particular that you would like approval >>from, >> be sure to add them in the To: of your email. >> - Again, sometimes this will end with a Lazy >> Consensus >> . >> >> Step 5: Merge your change >> - Once your topic branch is tested & working, it's time to merge it. >>Use >> the following workflow: >> >> git checkout master >> git pull apache master >> git checkout topic_branch >> git checkout -b to_be_merged >> git rebase master -i >> ... >> git checkout master >> git merge --ff-only to_be_merged >> git push apache master >> git branch -d to_be_merged >> git branch -D topic_branch >> git push apache :topic_branch >> >> The rebase -i step is your chance to clean up the commit messages and to >> combine small commits when appropriate. For example: >> Commit A: Implemented RockOn feature (CB-1234) >> Commit B: Added tests for RockOn (CB-1234) >> Commit C: Fixed RockOn not working with empty strings >> Commit D: Renamed RockOn to JustRock >> Commit E: Refactor MainFeature to make use of JustRock. >> >> In this case, it would be appropriate to combine commits A-D into a >>single >> commit, or at least commits A & C. Having a smaller number of commits >>when >> merging makes it easier for others to comprehend the diff commits, and >>also >> makes it easier to roll-back commits should the need arise. For JS >>commits, >> prefix the message with [platform] so that it's clear who should take >> interest in the commit. For all commits, be sure to include the JIRA >>issue >> number / URL. >> >> >> On Tue, Jan 22, 2013 at 1:19 PM, Braden Shepherdson > >wrote: >> >> > Code reviews will generally sound good to Googlers, so long as we can >> keep >> > the turnaround down. It definitely keeps our code quality high on >> internal >> > projects, even if it is sometimes a pain to have to wait for a >>response >> and >> > do your own reviews. I've asked Michal and Andrew for >>over-the-shoulder >> iOS >> > reviews in the past, since I'm new to that platform. >> > >> > I also want to apologize for the trouble with the ArrayBuffers on >> Android. >> > I was running into the bug with navigating in mobile-spec causing >> > deviceready not to fire, and had just changed my start page to the >>binary >> > echo test Michal wrote. It started working, so I cleaned up my >>debugging >> > and pushed. That was premature, since I broke some of the tests and >> hadn't >> > run the automatic tests. Gomen nasai. >> > >> > >> > Braden >> > >> > On Tue, Jan 22, 2013 at 10:59 AM, Andrew Grieve > > >wrote: >> > >> > > ReviewBoard seems like a great fit to me! Let's try it out! >> > > >> > > >> > > On Mon, Jan 21, 2013 at 8:43 PM, Brian M Dube >> wrote: >> > > >> > > > On 01/21/2013 01:24 PM, Joe Bowser wrote: >> > > > > On Mon, Jan 21, 2013 at 1:06 PM, Andrew Grieve < >> agrieve@chromium.org >> > > >> > > > wrote: >> > > > >> As for code reviews: >> > > > >> >> > > > >> I'd certainly be interested in more code-reviews. I think it's >> > really >> > > > >> useful to get feedback on changes. The only time when it >>becomes a >> > > > burden >> > > > >> is when turn-around time gets too long (e.g. you submit for >>review >> > and >> > > > no >> > > > >> one looks at it for over a day). >> > > > >> >> > > > >> Up until now, we've been using the github pull-request >>interface >> to >> > > have >> > > > >> others review our changes, but this isn't done very >>frequently. I >> > also >> > > > >> don't love this approach because comments through it don't get >> > posted >> > > > back >> > > > >> to the cordova mailing-list. >> > > > > >> > > > > I'm not super thrilled by this either, because our GitHub pull >> > request >> > > > > system is completely broken since we can't actually close >>requests >> > and >> > > > > indicate when we think things are a good idea or not. I think we >> > > > > should do what Android does with Gerrit (see >> > > > > https://android-review.googlesource.com) , but that'll involve >> > > > > additional infrastructure and another war with INFRA about >>whether >> > > > > it's the Apache way or whatever. >> > > > >> > > > An instance of ReviewBoard [1] exists at Apache [2], so I don't >>think >> > it >> > > > means war about the Apache way. Is that something that could fill >> this >> > > > need? >> > > > >> > > > Brian >> > > > >> > > > [1] https://reviews.apache.org/ >> > > > [2] >> > > > >> >=20 >>https://blogs.apache.org/infra/entry/reviewboard_instance_running_at_the >> > > > >> > > >> > >>