Mailing-List: contact dev-help@cordova.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cordova.apache.org
Received-SPF: pass (athena.apache.org: domain of fil@adobe.com designates
 64.18.1.25 as permitted sender)
From: Filip Maj <fil@adobe.com>
To: "dev@cordova.apache.org" <dev@cordova.apache.org>
Date: Tue, 22 Jan 2013 11:40:52 -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: Ac341/LvuACMqANQRzulQYsNfGWE7A==
Message-ID: <CD242B31.1640F%fil@adobe.com>
In-Reply-To: 
 <CAP7NMPpOaUxn6x2cxpWnvZHmZcVgTxWE+kj4gTe=FkbRB8COZg@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
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

Only runs against master but there is always work to be done :)

On 1/22/13 11:34 AM, "Michael Brooks" <michael@michaelbrooks.ca> wrote:

>>
>> Here's a draft for a new "Committing Your Own Changes:" section on
>> http://wiki.apache.org/cordova/CommitterWorkflow
>
>
>Great draft Andrew. It reads well.
>
>Outsider observation/note: probably useful for step 4 to tie into the CI -
>> getting the pull request to notify Fil's CI project.
>
>
>It would be nice to run a smoke test against the CI server. I'm unsure
>whether medic currently supports branches or only runs against master.
>
>On Tue, Jan 22, 2013 at 10:57 AM, Andrew Lunny <alunny@gmail.com> 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 <agrieve@chromium.org> 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
>><http://www.apache.org/foundation/glossary.html#LazyConsensus
>> >.
>> >
>> > 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
>> >=20
>>Consensus<http://www.apache.org/foundation/glossary.html#LazyConsensus>
>> > .
>> >
>> > 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
>><braden@chromium.org
>> > >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
>><agrieve@chromium.org
>> > > >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 <bdube@apache.org>
>> > 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]
>> > > > >
>> > >
>> https://blogs.apache.org/infra/entry/reviewboard_instance_running_at_the
>> > > > >
>> > > >
>> > >
>> >
>>