One tough thing about this is knowing when you have agreement or not. E.g. for adding File.slice(), Braden sent out an email on the 7th saying that he'd like to add it for iOS & Android. Simon gave it a +1, and no one else responded. He then implemented the feature in a feature branch and then merged it when it was working. Since there was an email with few replies, I gathered that it had been discussed, and that no one really had anything to say about it.
Joe - How do you propose that we make sure that things work? Or are you suggesting that a code-review serves this purpose? 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. Another example is just using feature branches: Last week I tried out the branch approach for the CB-2227. I sent an email saying what I wanted to do, created a branch, and sent my first "I've made progress" out on Wednesday. On Thursday, I responded to the thread saying that the initial work was done and asked for feedback. I got no feedback, so today I merged into Master and updated the bug. I did this because I feel pretty confident about the change, but also because it will force everyone to test it out, and we're still early in the current release-cycle. Maybe there's a problem with people not noticing when feedback is being requested? Should we have a more structured way of asking for feedback on a branch? E.g. Start a new thread on the ML with the subject "Review Request: ..." or something like that? And if you're proposing a new API, say "API Review: ...". WDYT? On Mon, Jan 21, 2013 at 2:12 PM, Jesse MacFadyen <purplecabb...@gmail.com>wrote: > Agree with both of you. Also of note is the ripple effect of adding a > feature to one platform, see the slice() discussion for an example. > Any new feature should be explored and discussed further than just > ios/android. > > > > Cheers, > Jesse > > Sent from my iPhone5 > > On 2013-01-21, at 10:50 AM, Brian LeRoux <b...@brian.io> wrote: > > Agree, but this should be just the regular flow rather than a formal > check in. Anything big should be in a branch and ideally ppl should > socialize branches on the list before the merge. > > On Mon, Jan 21, 2013 at 12:41 PM, Joe Bowser <bows...@gmail.com> wrote: > > Recently we've been noticing that there's been a lot of new features > > going into Cordova this release, especially in Android. Now, I know > > that I've been guilty of this in the past, which is partly why I'm > > saying this now. > > > > I think that we need to talk about features and make sure they work > > before we push them into the master repository. We can't add new > > methods that break how Cordova works for our older users without > > having a really good reason for it. We have time to actually review > > things before they arrive in the master branch, and it's trickier for > > us to pull a feature out of master once it's in there, especially when > > people commit to the branch after. That's why I think we should have > > a review process in place. > > > > What are people's thoughts on this? > > > > Joe >
