cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Brooks <mich...@michaelbrooks.ca>
Subject Re: Can we talk about large features before they arrive in master?
Date Tue, 22 Jan 2013 19:34:46 GMT
>
> 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 <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
> > 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
> > > > >
> > > >
> > >
> >
>

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