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 <bra...@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 <agri...@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 <bd...@apache.org> wrote:
> >
> > > On 01/21/2013 01:24 PM, Joe Bowser wrote:
> > > > On Mon, Jan 21, 2013 at 1:06 PM, Andrew Grieve <agri...@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
> > >
> >
>
