> > 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 <alu...@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 <agri...@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 <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 > > > > > > > > > > > > > > >
