We need better contributions from new people

[Joe Bowser <bo...@gmail.com>]

Hey

I decided to check my e-mail and respond to an issue.  It turns out
that we're not properly testing or even checking the pull requests
that we're getting into the project. The bug in question is CB-3766,
which was caused by a patch accepted to fix CB-2458.  The error isn't
totally obvious, and doesn't get easily picked up on unit test (our
CordovaWebView test mostly works, but fires a TIMING ERROR), but if
you use an emulator, due to the emulator's crappiness, it apparently
creates an ugly stack trace.

Honestly, there's no way that this should have made it in.  I
personally think that I'm going to have to rename loadUrlIntoView to
initAndLoadUrl or something more obvious, since it's too similar to
loadUrl.  That being said, we need to be more strict about quality
while at the same time encouraging people to contribute.  I'd rather
deal with hundreds of pull requests than hundreds of issues where
people know the answer but don't tell you.

Any ideas how we can accomplish this?  Has anyone else seen pull
requests get in that shouldn't have?

Joe

Re: We need better contributions from new people

[Marcel Kinard <cm...@gmail.com>]

That sounds good.

For the automated tests, it would seem that running all of them should be the default since it takes basically no extra effort to do so. If that sounds reasonable, how about a tweak on your tweak: 

- When we say "run the test suites" this includes all automated tests in mobile-spec, manual tests in mobile-spec that might be affected by the change, and any platform-specific unit tests (i.e., cordova-android/test, cordova-ios/CordovaLibTests, etc.)

On Jun 17, 2013, at 10:31 AM, Andrew Grieve <ag...@chromium.org> wrote:

> Marcel, I like your wiki suggestion. I think it'll capture Joe's concern to
> just change:
> 
> - When we say "run the test suites" this includes the automated tests in
> mobile-spec, the manual tests in mobile-spec and any platform-specific unit
> tests (i.e., cordova-android/test, cordova-ios/CordovaLibTests, etc.)
> 
> to:
> 
> - When we say "run the test suites" this includes all automated and manual
> tests in mobile-spec that might be affected by the change, and any
> platform-specific unit tests (i.e., cordova-android/test,
> cordova-ios/CordovaLibTests, etc.)
> 
> 
> On Thu, Jun 13, 2013 at 7:58 PM, Marcel Kinard <cm...@gmail.com> wrote:
> 
>> On Jun 13, 2013, at 11:16 AM, Joe Bowser <bo...@gmail.com> wrote:
>> 
>>> No offense, but have you ever run all the manual tests? Doing this is a
>>> time consuming process and is unnecessary in most cases.
>> 
>> (It takes a lot to offend me, you're fine.)
>> 
>> Agree on being time consuming, compared to the automated tests. And
>> sometimes it's not entirely clear when a manual test succeeds/fails.
>> 
>> Yeah, the full manual tests are probably overkill on a per-commit basis.
>> So when should they get run, just as a final pass when the RC is cut? Or
>> some subset of "applicable" ones that intersect with the function that is
>> being touched in a commit? Or some other trigger?
>> 
>>> You can write what you want on the wiki but
>>> it doesn't mean the people writing the code will follow it (myself
>>> included).
>> 
>> My goal here is not to put verbage on the wiki, but to get consensus on
>> the standard procedure on this topic. The reason I'm asking these questions
>> is it sounds like they haven't been talked about at this level of detail
>> before, otherwise I would be getting schooled right now. ;-)
>> 
>> -- Marcel Kinard

Re: We need better contributions from new people

[Andrew Grieve <ag...@chromium.org>]

Marcel, I like your wiki suggestion. I think it'll capture Joe's concern to
just change:

- When we say "run the test suites" this includes the automated tests in
mobile-spec, the manual tests in mobile-spec and any platform-specific unit
tests (i.e., cordova-android/test, cordova-ios/CordovaLibTests, etc.)

to:

- When we say "run the test suites" this includes all automated and manual
tests in mobile-spec that might be affected by the change, and any
platform-specific unit tests (i.e., cordova-android/test,
cordova-ios/CordovaLibTests, etc.)


On Thu, Jun 13, 2013 at 7:58 PM, Marcel Kinard <cm...@gmail.com> wrote:

> On Jun 13, 2013, at 11:16 AM, Joe Bowser <bo...@gmail.com> wrote:
>
> > No offense, but have you ever run all the manual tests? Doing this is a
> > time consuming process and is unnecessary in most cases.
>
> (It takes a lot to offend me, you're fine.)
>
> Agree on being time consuming, compared to the automated tests. And
> sometimes it's not entirely clear when a manual test succeeds/fails.
>
> Yeah, the full manual tests are probably overkill on a per-commit basis.
> So when should they get run, just as a final pass when the RC is cut? Or
> some subset of "applicable" ones that intersect with the function that is
> being touched in a commit? Or some other trigger?
>
> > You can write what you want on the wiki but
> > it doesn't mean the people writing the code will follow it (myself
> > included).
>
> My goal here is not to put verbage on the wiki, but to get consensus on
> the standard procedure on this topic. The reason I'm asking these questions
> is it sounds like they haven't been talked about at this level of detail
> before, otherwise I would be getting schooled right now. ;-)
>
> -- Marcel Kinard

Re: We need better contributions from new people

[Marcel Kinard <cm...@gmail.com>]

On Jun 13, 2013, at 11:16 AM, Joe Bowser <bo...@gmail.com> wrote:

> No offense, but have you ever run all the manual tests? Doing this is a
> time consuming process and is unnecessary in most cases.

(It takes a lot to offend me, you're fine.)

Agree on being time consuming, compared to the automated tests. And sometimes it's not entirely clear when a manual test succeeds/fails.

Yeah, the full manual tests are probably overkill on a per-commit basis. So when should they get run, just as a final pass when the RC is cut? Or some subset of "applicable" ones that intersect with the function that is being touched in a commit? Or some other trigger?

> You can write what you want on the wiki but
> it doesn't mean the people writing the code will follow it (myself
> included).

My goal here is not to put verbage on the wiki, but to get consensus on the standard procedure on this topic. The reason I'm asking these questions is it sounds like they haven't been talked about at this level of detail before, otherwise I would be getting schooled right now. ;-)

-- Marcel Kinard

Re: We need better contributions from new people

[Joe Bowser <bo...@gmail.com>]

No offense, but have you ever run all the manual tests? Doing this is a
time consuming process and is unnecessary in most cases.

We automated both mobile-spec and the JUnit tests so that they get run by
developers on a regular basis. You can write what you want on the wiki but
it doesn't mean the people writing the code will follow it (myself
included).

All I'm asking for is some testing, not all the tests to be run. Going from
one extreme to another isn't going to be helpful.
On Jun 13, 2013 8:02 AM, "Marcel Kinard" <cm...@gmail.com> wrote:

> On Jun 12, 2013, at 6:10 PM, Joe Bowser <bo...@gmail.com> wrote:
>
> > The problem with this approach is that we shouldn't rely on just
> > mobile-spec for our testing...
> >
> > The fact is that on Android, if you're doing changes to the core, you
> > should be running the JUnit tests included with the project...
>
>
> Sounds good. So how about the following:
>
> - the developer (contributor or committer) is responsible to test their
> own code and correct any problems before a pull request is submitted
> (contributor authored) or it lands in the stream (committer authored). The
> testing includes both verifying the function they added/touched, plus
> running the test suites to verify there are no regressions.
> - When we say "run the test suites" this includes the automated tests in
> mobile-spec, the manual tests in mobile-spec and any platform-specific unit
> tests (i.e., cordova-android/test, cordova-ios/CordovaLibTests, etc.)
> - if a pull request, the committer should sanity check the contributor's
> code. This should include at least a simple visual inspection.
> - if a pull request, the committer should see that the contributor tested
> it. A simple comment in Jira should suffice, similar to how Mike and Lisa
> worked as contributors here: https://issues.apache.org/jira/browse/CB-3521
>
> If there is consensus, I can add the above verbage to the wiki.
>
> Do we want to go as far as the following?
>
> - every new function and defect fix should include an automated test
> (where reasonably feasible) that lands in a test suite at the same time as
> the new function / fix.
>
>
>

Re: We need better contributions from new people

[Marcel Kinard <cm...@gmail.com>]

On Jun 12, 2013, at 6:10 PM, Joe Bowser <bo...@gmail.com> wrote:

> The problem with this approach is that we shouldn't rely on just
> mobile-spec for our testing...
> 
> The fact is that on Android, if you're doing changes to the core, you
> should be running the JUnit tests included with the project...


Sounds good. So how about the following:

- the developer (contributor or committer) is responsible to test their own code and correct any problems before a pull request is submitted (contributor authored) or it lands in the stream (committer authored). The testing includes both verifying the function they added/touched, plus running the test suites to verify there are no regressions.
- When we say "run the test suites" this includes the automated tests in mobile-spec, the manual tests in mobile-spec and any platform-specific unit tests (i.e., cordova-android/test, cordova-ios/CordovaLibTests, etc.)
- if a pull request, the committer should sanity check the contributor's code. This should include at least a simple visual inspection.
- if a pull request, the committer should see that the contributor tested it. A simple comment in Jira should suffice, similar to how Mike and Lisa worked as contributors here: https://issues.apache.org/jira/browse/CB-3521 

If there is consensus, I can add the above verbage to the wiki.

Do we want to go as far as the following?

- every new function and defect fix should include an automated test (where reasonably feasible) that lands in a test suite at the same time as the new function / fix. 

Re: We need better contributions from new people

[Joe Bowser <bo...@gmail.com>]

The problem with this approach is that we shouldn't rely on just
mobile-spec for our testing. In this example, I wasn't able to fully
reproduce what he was doing because the person who filed it refused to
provide me with a project.  That being said, I was able to find that I
was getting weird TIMEOUT errors with the code due to it going through
the wrong method.  While on my devices, it was a really minor error in
logcat that would be easily ignored, it apparently causes meltdowns on
the emulator.

The fact is that on Android, if you're doing changes to the core, you
should be running the JUnit tests incldued with the project.  These
tests are completely independent of mobile-spec and catch a majority
of the native use cases that we currently have running.  mobile-spec
is the cross-platform test suite, and every platform should have its
own native test suite to test its internals to make sure we're not
totally screwing things up.  If there's an error from Cordova that
appears after running it, chances are that something is wrong, even if
the tests pass.  Remember that for certain bugs, like bugs where you
exit the app, which this one was, it's really hard to test those even
with JUnit.

As far as this being a recurring problem, I do think that it is
starting recurring problem, since our users are saying that they
haven't seen a quality release of Cordova-Android since 2.4.  That
means that we broke something in the past four months that meant that
they couldn't upgrade.  Some of it was stuff we couldn't avoid, like
the WebSQL vs WebStorage choice, or deprecations of Plugins.java, but
most of it was stuff like this and the Splashscreen error, where we'd
accept a patch without testing.

So, please write more JUnit tests as well as mobile-spec.  mobile-spec
is not a silver bullet for testing on Cordova, since it really only
tests Cordova JS and the API.

On Wed, Jun 12, 2013 at 2:07 PM, Marcel Kinard <cm...@gmail.com> wrote:
> +1 to Ian's comments.
>
> How about if:
> - the developer (contributor or committer) is responsible to test their own code and correct any problems before a pull request is submitted (contributor authored) or it lands in the stream (committer authored). The testing includes both verifying the function they added/touched, plus running mobile-spec to verify there are no regressions.
> - if a pull request, the committer should sanity check the contributor's code.
> - if a pull request, the committer should see that the contributor tested it. Perhaps that could be a simple comment in Jira, similar to how Mike and Lisa worked as contributors here: https://issues.apache.org/jira/browse/CB-3521 .
>
> I find it odd that "Processing Pull Requests" in http://wiki.apache.org/cordova/CommitterWorkflow doesn't mention anything about testing or sanity checking / reviewing what comes from the contributor. If there is consensus, I can add the verbage above to "Processing Pull Requests". (Hmm, shouldn't there also be something there about doc for external behaviors / interfaces / requirements?)
>
> Based on other commercial projects I've seen, and how aggressive you want to be, I'd also be tempted to suggest:
> - every new function and defect fix should include an automated test (where reasonably feasible) that lands in mobile-spec at the same time as the new function / fix. What this does over time is to build up coverage in the automated test suite that gets a lot better at catching breakages / regressions. Yes, this takes time to do, but when mobile-spec is run, you've got confidence.
>
> -- Marcel Kinard

Re: We need better contributions from new people

[Marcel Kinard <cm...@gmail.com>]

+1 to Ian's comments.

How about if:
- the developer (contributor or committer) is responsible to test their own code and correct any problems before a pull request is submitted (contributor authored) or it lands in the stream (committer authored). The testing includes both verifying the function they added/touched, plus running mobile-spec to verify there are no regressions.
- if a pull request, the committer should sanity check the contributor's code. 
- if a pull request, the committer should see that the contributor tested it. Perhaps that could be a simple comment in Jira, similar to how Mike and Lisa worked as contributors here: https://issues.apache.org/jira/browse/CB-3521 .

I find it odd that "Processing Pull Requests" in http://wiki.apache.org/cordova/CommitterWorkflow doesn't mention anything about testing or sanity checking / reviewing what comes from the contributor. If there is consensus, I can add the verbage above to "Processing Pull Requests". (Hmm, shouldn't there also be something there about doc for external behaviors / interfaces / requirements?)

Based on other commercial projects I've seen, and how aggressive you want to be, I'd also be tempted to suggest:
- every new function and defect fix should include an automated test (where reasonably feasible) that lands in mobile-spec at the same time as the new function / fix. What this does over time is to build up coverage in the automated test suite that gets a lot better at catching breakages / regressions. Yes, this takes time to do, but when mobile-spec is run, you've got confidence.

-- Marcel Kinard

RE: We need better contributions from new people

[Ken Wallis <kw...@blackberry.com>]

It is important to ensure that code coming in is good quality, I think we can all appreciate that.

I don't think that is separate or instead of the ability to be agile, which we also see as a positive for this community. From what I have read on this thread, it sounds like our ability to be agile and address bugs quickly might be hampered by not having efficient mechanisms for getting feedback and bugs back from the user base?

--

Ken Wallis

Product Manager – WebWorks

BlackBerry

289-261-4369

________________________________________
From: maxw@google.com [maxw@google.com] on behalf of Max Woghiren [maxw@chromium.org]
Sent: Wednesday, June 12, 2013 7:35 AM
To: dev
Subject: Re: We need better contributions from new people

I agree with Ian—reviewing and testing just needs to be more thorough.


On Wed, Jun 12, 2013 at 10:17 AM, Michal Mocny <mm...@chromium.org> wrote:

> Before we go changing process, is this really a systemic issue?  Bugs
> happen, and people sometimes land code, I'de rather be nimble in fixing
> those issues than rigid in preventing them.  Personal opinion.
>
> -Michal
>
>
> On Wed, Jun 12, 2013 at 9:54 AM, Ian Clelland <iclelland@chromium.org
> >wrote:
>
> > That's not an issue of "we need better contributions from new people",
> > that's "We, the committers, need to be more diligent in reviewing and
> > testing before committing".
> >
> > We shouldn't be discouraging contributions from anyone, but we don't have
> > to accept every pull request as-is, either. We can push back on the patch
> > if it isn't clean enough, or if it isn't clear that it's the right
> > solution, or even if it doesn't come with tests / docs.
> >
> > I'd have no problem with a lot more pull requests, if there was healthy
> > discussion on JIRA for each one, including the core committers and the
> > original reporter, before accepting (or rejecting) them.
> >
> > Regarding this particular commit, the original patch is ugly, and
> probably
> > shouldn't have been accepted without some cleanup, review, and attached
> > tests. The additional issue that it caused, though, is obviously
> something
> > that wasn't covered by our existing tests. It was only noticed a couple
> of
> > months later, in a post on StackOverflow, and as far as I can tell, it
> was
> > only mentioned in Github 18 days ago, and Andrew was right on it. (He's
> not
> > working on it currently, for other reasons)
> >
> > That regression doesn't seem to me to be a failure in process, so much
> as a
> > gap in our test framework (which we can rectify easily enough) and maybe
> an
> > issue of not-paying-enough-attention-to-stackoverflow, or
> > not-having-a-clear-channel-to-report-bugs. Maybe if it were easier for
> > people to tell us when something broke and we didn't catch it, we might
> > have fixed this a month sooner.
> >
> >
> >
> > On Wed, Jun 12, 2013 at 2:09 AM, Joe Bowser <bo...@gmail.com> wrote:
> >
> > > Hey
> > >
> > > I decided to check my e-mail and respond to an issue.  It turns out
> > > that we're not properly testing or even checking the pull requests
> > > that we're getting into the project. The bug in question is CB-3766,
> > > which was caused by a patch accepted to fix CB-2458.  The error isn't
> > > totally obvious, and doesn't get easily picked up on unit test (our
> > > CordovaWebView test mostly works, but fires a TIMING ERROR), but if
> > > you use an emulator, due to the emulator's crappiness, it apparently
> > > creates an ugly stack trace.
> > >
> > > Honestly, there's no way that this should have made it in.  I
> > > personally think that I'm going to have to rename loadUrlIntoView to
> > > initAndLoadUrl or something more obvious, since it's too similar to
> > > loadUrl.  That being said, we need to be more strict about quality
> > > while at the same time encouraging people to contribute.  I'd rather
> > > deal with hundreds of pull requests than hundreds of issues where
> > > people know the answer but don't tell you.
> > >
> > > Any ideas how we can accomplish this?  Has anyone else seen pull
> > > requests get in that shouldn't have?
> > >
> > > Joe
> > >
> >
>

---------------------------------------------------------------------
This transmission (including any attachments) may contain confidential information, privileged material (including material protected by the solicitor-client or other applicable privileges), or constitute non-public information. Any use of this information by anyone other than the intended recipient is prohibited. If you have received this transmission in error, please immediately reply to the sender and delete this information from your system. Use, dissemination, distribution, or reproduction of this transmission by unintended recipients is not authorized and may be unlawful.

Re: We need better contributions from new people

[Max Woghiren <ma...@chromium.org>]

I agree with Ian—reviewing and testing just needs to be more thorough.


On Wed, Jun 12, 2013 at 10:17 AM, Michal Mocny <mm...@chromium.org> wrote:

> Before we go changing process, is this really a systemic issue?  Bugs
> happen, and people sometimes land code, I'de rather be nimble in fixing
> those issues than rigid in preventing them.  Personal opinion.
>
> -Michal
>
>
> On Wed, Jun 12, 2013 at 9:54 AM, Ian Clelland <iclelland@chromium.org
> >wrote:
>
> > That's not an issue of "we need better contributions from new people",
> > that's "We, the committers, need to be more diligent in reviewing and
> > testing before committing".
> >
> > We shouldn't be discouraging contributions from anyone, but we don't have
> > to accept every pull request as-is, either. We can push back on the patch
> > if it isn't clean enough, or if it isn't clear that it's the right
> > solution, or even if it doesn't come with tests / docs.
> >
> > I'd have no problem with a lot more pull requests, if there was healthy
> > discussion on JIRA for each one, including the core committers and the
> > original reporter, before accepting (or rejecting) them.
> >
> > Regarding this particular commit, the original patch is ugly, and
> probably
> > shouldn't have been accepted without some cleanup, review, and attached
> > tests. The additional issue that it caused, though, is obviously
> something
> > that wasn't covered by our existing tests. It was only noticed a couple
> of
> > months later, in a post on StackOverflow, and as far as I can tell, it
> was
> > only mentioned in Github 18 days ago, and Andrew was right on it. (He's
> not
> > working on it currently, for other reasons)
> >
> > That regression doesn't seem to me to be a failure in process, so much
> as a
> > gap in our test framework (which we can rectify easily enough) and maybe
> an
> > issue of not-paying-enough-attention-to-stackoverflow, or
> > not-having-a-clear-channel-to-report-bugs. Maybe if it were easier for
> > people to tell us when something broke and we didn't catch it, we might
> > have fixed this a month sooner.
> >
> >
> >
> > On Wed, Jun 12, 2013 at 2:09 AM, Joe Bowser <bo...@gmail.com> wrote:
> >
> > > Hey
> > >
> > > I decided to check my e-mail and respond to an issue.  It turns out
> > > that we're not properly testing or even checking the pull requests
> > > that we're getting into the project. The bug in question is CB-3766,
> > > which was caused by a patch accepted to fix CB-2458.  The error isn't
> > > totally obvious, and doesn't get easily picked up on unit test (our
> > > CordovaWebView test mostly works, but fires a TIMING ERROR), but if
> > > you use an emulator, due to the emulator's crappiness, it apparently
> > > creates an ugly stack trace.
> > >
> > > Honestly, there's no way that this should have made it in.  I
> > > personally think that I'm going to have to rename loadUrlIntoView to
> > > initAndLoadUrl or something more obvious, since it's too similar to
> > > loadUrl.  That being said, we need to be more strict about quality
> > > while at the same time encouraging people to contribute.  I'd rather
> > > deal with hundreds of pull requests than hundreds of issues where
> > > people know the answer but don't tell you.
> > >
> > > Any ideas how we can accomplish this?  Has anyone else seen pull
> > > requests get in that shouldn't have?
> > >
> > > Joe
> > >
> >
>

Re: We need better contributions from new people

[Michal Mocny <mm...@chromium.org>]

Before we go changing process, is this really a systemic issue?  Bugs
happen, and people sometimes land code, I'de rather be nimble in fixing
those issues than rigid in preventing them.  Personal opinion.

-Michal


On Wed, Jun 12, 2013 at 9:54 AM, Ian Clelland <ic...@chromium.org>wrote:

> That's not an issue of "we need better contributions from new people",
> that's "We, the committers, need to be more diligent in reviewing and
> testing before committing".
>
> We shouldn't be discouraging contributions from anyone, but we don't have
> to accept every pull request as-is, either. We can push back on the patch
> if it isn't clean enough, or if it isn't clear that it's the right
> solution, or even if it doesn't come with tests / docs.
>
> I'd have no problem with a lot more pull requests, if there was healthy
> discussion on JIRA for each one, including the core committers and the
> original reporter, before accepting (or rejecting) them.
>
> Regarding this particular commit, the original patch is ugly, and probably
> shouldn't have been accepted without some cleanup, review, and attached
> tests. The additional issue that it caused, though, is obviously something
> that wasn't covered by our existing tests. It was only noticed a couple of
> months later, in a post on StackOverflow, and as far as I can tell, it was
> only mentioned in Github 18 days ago, and Andrew was right on it. (He's not
> working on it currently, for other reasons)
>
> That regression doesn't seem to me to be a failure in process, so much as a
> gap in our test framework (which we can rectify easily enough) and maybe an
> issue of not-paying-enough-attention-to-stackoverflow, or
> not-having-a-clear-channel-to-report-bugs. Maybe if it were easier for
> people to tell us when something broke and we didn't catch it, we might
> have fixed this a month sooner.
>
>
>
> On Wed, Jun 12, 2013 at 2:09 AM, Joe Bowser <bo...@gmail.com> wrote:
>
> > Hey
> >
> > I decided to check my e-mail and respond to an issue.  It turns out
> > that we're not properly testing or even checking the pull requests
> > that we're getting into the project. The bug in question is CB-3766,
> > which was caused by a patch accepted to fix CB-2458.  The error isn't
> > totally obvious, and doesn't get easily picked up on unit test (our
> > CordovaWebView test mostly works, but fires a TIMING ERROR), but if
> > you use an emulator, due to the emulator's crappiness, it apparently
> > creates an ugly stack trace.
> >
> > Honestly, there's no way that this should have made it in.  I
> > personally think that I'm going to have to rename loadUrlIntoView to
> > initAndLoadUrl or something more obvious, since it's too similar to
> > loadUrl.  That being said, we need to be more strict about quality
> > while at the same time encouraging people to contribute.  I'd rather
> > deal with hundreds of pull requests than hundreds of issues where
> > people know the answer but don't tell you.
> >
> > Any ideas how we can accomplish this?  Has anyone else seen pull
> > requests get in that shouldn't have?
> >
> > Joe
> >
>

Re: We need better contributions from new people

[Ian Clelland <ic...@chromium.org>]

That's not an issue of "we need better contributions from new people",
that's "We, the committers, need to be more diligent in reviewing and
testing before committing".

We shouldn't be discouraging contributions from anyone, but we don't have
to accept every pull request as-is, either. We can push back on the patch
if it isn't clean enough, or if it isn't clear that it's the right
solution, or even if it doesn't come with tests / docs.

I'd have no problem with a lot more pull requests, if there was healthy
discussion on JIRA for each one, including the core committers and the
original reporter, before accepting (or rejecting) them.

Regarding this particular commit, the original patch is ugly, and probably
shouldn't have been accepted without some cleanup, review, and attached
tests. The additional issue that it caused, though, is obviously something
that wasn't covered by our existing tests. It was only noticed a couple of
months later, in a post on StackOverflow, and as far as I can tell, it was
only mentioned in Github 18 days ago, and Andrew was right on it. (He's not
working on it currently, for other reasons)

That regression doesn't seem to me to be a failure in process, so much as a
gap in our test framework (which we can rectify easily enough) and maybe an
issue of not-paying-enough-attention-to-stackoverflow, or
not-having-a-clear-channel-to-report-bugs. Maybe if it were easier for
people to tell us when something broke and we didn't catch it, we might
have fixed this a month sooner.



On Wed, Jun 12, 2013 at 2:09 AM, Joe Bowser <bo...@gmail.com> wrote:

> Hey
>
> I decided to check my e-mail and respond to an issue.  It turns out
> that we're not properly testing or even checking the pull requests
> that we're getting into the project. The bug in question is CB-3766,
> which was caused by a patch accepted to fix CB-2458.  The error isn't
> totally obvious, and doesn't get easily picked up on unit test (our
> CordovaWebView test mostly works, but fires a TIMING ERROR), but if
> you use an emulator, due to the emulator's crappiness, it apparently
> creates an ugly stack trace.
>
> Honestly, there's no way that this should have made it in.  I
> personally think that I'm going to have to rename loadUrlIntoView to
> initAndLoadUrl or something more obvious, since it's too similar to
> loadUrl.  That being said, we need to be more strict about quality
> while at the same time encouraging people to contribute.  I'd rather
> deal with hundreds of pull requests than hundreds of issues where
> people know the answer but don't tell you.
>
> Any ideas how we can accomplish this?  Has anyone else seen pull
> requests get in that shouldn't have?
>
> Joe
>