cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Grieve <agri...@chromium.org>
Subject Re: Can we talk about large features before they arrive in master?
Date Mon, 21 Jan 2013 21:50:51 GMT
On Mon, Jan 21, 2013 at 4:24 PM, Joe Bowser <bowserj@gmail.com> wrote:

> On Mon, Jan 21, 2013 at 1:06 PM, Andrew Grieve <agrieve@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.
>

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