cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Bowser <bows...@gmail.com>
Subject Re: Can we talk about large features before they arrive in master?
Date Mon, 21 Jan 2013 22:18:35 GMT
On Mon, Jan 21, 2013 at 2:13 PM, Filip Maj <fil@adobe.com> wrote:
>
>>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.
>
> Yeah let's summarize our discussion points / rules of thumb down in the
> wiki page in case we were missing anything there.
>
> TBH we haven't done many code reviews in the past. I believe
> CordovaWebView was one of the biggest features we landed since about
> Cordova 1.0 that we had actually organized a code review, set up a
> conference call and screenshare to join in, etc. It lived in a branch I
> believe through two or three point releases just because it was a giant
> feature, needed a couple rebases/merges, and failed the first couple times
> we tried to merge it in.
>

I feel that this was to excessive, and that it actually hurt us in the
long-run having CordovaWebView in review for so long.  That being
said, it did teach us what's important to watch out for when
introducing new features, and what features will cause our users to
want our heads on sticks.  I think an informal review is good enough.
I usually wait 24 to 48 hours before merging things in. That being
said, I'm able to amuse myself with weird side projects like Purity
(https://github.com/infil00p/cordova-android/tree/puritytool) and
other tasks while I wait. :P

> In general, committers are trusted to be working on features that are
> important to the project and will land high quality code eventually into
> master. We just need that extra step of discipline of running mobile-spec
> and running the native tests for affected platforms on a few devices for
> sanity's sake. And as always, if you can't do this for whatever reason,
> you can always reach out to the list and ask for other committers to test
> out feature x on device y, or even just a smoke/sanity test on another
> committer's machine, etc. before merging into master.
>

Agreed! We have a lack of old Android 2.3 devices in the Vancouver
office, and a lot of Android 4.x devices.  I'll usually need to get
things tested on the original Samsung Galaxy S or some other Samsung
Galaxy phone that's not the top of the line, or is ruggedized or
something (i.e. Samsung Galaxy Rugby).

Mime
View raw message