On Mon, Jan 21, 2013 at 4:24 PM, Joe Bowser <bows...@gmail.com> wrote:
> On Mon, Jan 21, 2013 at 1:06 PM, Andrew Grieve <agri...@chromium.org> > wrote: > > 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? > > > > I do think a code review serves this purpose. I need to see a > published branch somewhere that I can test to make sure that things > don't break on my end before I can say "Hey, this looks good!", and > I'd prefer if mobile-spec was updated as part of the feature so I > don't have to figure out what's being done myself. > > > 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. > > > > > 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. > > > > I think this is fine and is exactly what needs to happen. It didn't > seem to break anything ,and there was a branch that we could look at. > The thing is that I don't remember seeing a branch for ArrayBuffer > that we could look at. I'll dig through my e-mail to find it, but I > don't remember seeing it. > Braden just used a private branch and didn't ask for review. Maybe the take-away from this is that we it would be good to have the chance for a review for anything that was big enough to warrant a branch? I'll add this to the committer's wiki page. > > > 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? > > > > I think that having a proper header on the e-mail would be a good plan. >
