cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jesse <>
Subject Re: Android JUnit Tests Now Pass
Date Thu, 12 Feb 2015 17:31:30 GMT
This commit may not have warranted this discussion. 
I think we agree that large changes/commits should be on feature branches, and discussed before
being merged. 
Let's go with that. 

> On Feb 12, 2015, at 8:49 AM, Andrew Grieve <> wrote:
> Sounds like you've been having a rough time. :( Hope you get through it.
> Believe me when I say I hear you loud and clear about making changes on
> feature branches. I just don't think this one fits.
> - No one (other than me) has touched the tests since September of last
> year, so it's unlikely that a change would cause merge conflicts.
> - The state of the tests show that they are not viewed as that important
> (at least not important enough for anyone other than me to have been
> working on them)
> - Anything I do to them doesn't affect shipping code. No risk.
> I find it hard to believe that my making changes contributes in a
> significant way to having people avoid working on Android. Perhaps being
> overly abrasive via email & JIRA would be a deterrent though...
>> On Thu, Feb 12, 2015 at 11:10 AM, Joe Bowser <> wrote:
>> On Thu Feb 12 2015 at 7:44:52 AM Andrew Grieve <>
>> wrote:
>>> I agree that significant changes should be reviewed first. But for the
>> most
>>> part Cordova is a review-after-commit kind of place,
>> No, it's not.  Cordova is only like that because you consistently make it
>> like that.  Constantly committing to master without any review at all makes
>> it next to impossible for anyone else to work on the project.  You can tell
>> that this is the case, because you own the majority of the commits over the
>> last few months. That's not normal for a codebase this size with this many
>> contributors.  This is why we have topic branches, and we've had this
>> discussion with you numerous times about using them.  This is also why I
>> write e-mails trying to get buy-in to what I want to do instead of just
>> landing features straight on master that could break everything.
>>> and this change didn't
>>> touch any code that we release (strictly tests... that have been broken
>> for
>>> a very long time), so I don't think it qualifies.
>> I'll admit that the tests were a bit of the wild west.  That said, there
>> was always an understanding that tests would be added to and improved upon
>> and not removed.  Marcel and I probably wouldn't have had half the e-mails
>> that we have had if it wasn't us arguing over whether to delete tests.
>> I know it's frustrating to have to wait on other people, since people are
>> human, get sick, and have to take care of others when they're sick.  That
>> said, it's equally frustrating to come back from vacation, or wake up from
>> a nap after driving someone from the hospital and see that critical code
>> that was a major issue only six months ago got accidentally removed in a
>> sweeping change, along with other use cases.  That's what happened
>> yesterday, and that's why I got frustrated.  If I'm having a bad day
>> already, a random refactor that just gets dropped without at least a head's
>> up beforehand makes it worse.
>> I've been on both sides of the issue with this.  I remember getting
>> extremely frustrated with Bryce when we designed CordovaWebView, especially
>> since my design had less of a change to the code.  I thought things were
>> moving too slowly, but at the end of the day we did produce something that
>> a lot of people seem to use (at least that's what the sample repo I have on
>> GitHub tells me, the GitHub analytics tools are all I have to go on). That
>> said, we didn't ship that until it was mostly ready, and other than an
>> awkward presentation at PhoneGap Day, it was mostly fine.  I'm glad I
>> didn't just merge my crap in and just unilaterally introduce that feature,
>> since back then we could still get away with that technically.
>> But yeah, can we have things on feature branches if they're that big, and
>> then wait maybe 24 hours before dropping something like that? I'm not
>> talking like a simple JIRA fix, but something that large should have been a
>> pull request or on its own branch or something.
>>>> On Thu, Feb 12, 2015 at 4:07 AM, Jesse <> wrote:
>>>> You may or may not, but I think it would be nice to let others review
>>> your
>>>> (significant) changes before dumping them to master.
>>>>> On Feb 11, 2015, at 6:34 PM, Andrew Grieve <>
>>> wrote:
>>>>>> On Wed, Feb 11, 2015 at 5:00 PM, Jesse <>
>>> wrote:
>>>>>> +1 Revert
>>>>>> And please let's stop deleting what other people wrote just because
>> we
>>>>>> don't recognize it. These things should require discussion.
>>>>> Bit of a jump to conclusions, don't you think? What makes you think I
>>>> don't
>>>>> recognize the code I changed?
>>>>>> @purplecabbage
>>>>>>> On Wed, Feb 11, 2015 at 1:53 PM, Joe Bowser <>
>>> wrote:
>>>>>>> I think we should revert this refactor.  With the new refactored
>>> tests,
>>>>>>> they may pass but we lost a lot of the useful tests that we once
>> had
>>>> and
>>>>>>> these new tests have no value.  I don't know why you took it
>>>>>> yourself
>>>>>>> to throw away all the JUnit tests that didn't pass, but that
>>> the
>>>>>>> point.  I would have rather had the old tests expanded upon instead
>>> of
>>>>>> just
>>>>>>> deleted on your personal whim.
>>>>>>> I honestly don't know what to say, I know that we have a terrible
>>>> working
>>>>>>> relationship at best, but this actually is making the project
>>>>>>> intentionally for unknown reasons.  In fact, I would almost say
>> that
>>>> this
>>>>>>> is purely a malicious change driven by ego, since I can't see
>>>> technical
>>>>>>> reason for any of it.
>>>>>>>> On Wed Feb 11 2015 at 1:36:19 PM Joe Bowser <>
>>>> wrote:
>>>>>>>> I think there's a lot of value in the Unit Tests, having
wrote the
>>>>>>>> majority of them initially.  If I wasn't dealing with everyone
>> my
>>>>>>> house
>>>>>>>> getting sick, I'd check to make sure these tests were still
>> testing
>>>>>> what
>>>>>>> I
>>>>>>>> intended them to test, since we have a habit of losing the
>>>>>> behind
>>>>>>>> the test every time we do a refactor.
>>>>>>>> Of course, if we're going to throw away the embedded WebView
>>>> then
>>>>>>>> maybe there's not value after all.
>>>>>>>> On Wed Feb 11 2015 at 1:12:29 PM Andrew Grieve <
>>>>>>>> wrote:
>>>>>>>>> Does travis provide Android emulators? I'd guess it'd
be too slow
>>> to
>>>>>> put
>>>>>>>>> on
>>>>>>>>> Travis. And honestly, there's still not a lot of value
in the
>> unit
>>>>>> tests
>>>>>>>>> atm.
>>>>>>>>> On Wed, Feb 11, 2015 at 3:12 PM, Murat Sutunc <
>>>>>>>>> wrote:
>>>>>>>>>> This is great news!
>>>>>>>>>> I've finally got the android travis enabled too.
We have jshint
>>> and
>>>>>>>>>> jasmine test coverage on every commit now. (
>>>>>>>>>> Now that we're passing all junit tests, I think the
next step
>> for
>>> us
>>>>>>>>>> should be to integrate junit tests with travis. What
do you
>> think?
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: []
On Behalf
>> Of
>>>>>>>>> Andrew
>>>>>>>>>> Grieve
>>>>>>>>>> Sent: Tuesday, February 10, 2015 7:14 PM
>>>>>>>>>> To: dev
>>>>>>>>>> Subject: Android JUnit Tests Now Pass
>>>>>>>>>> Spent some time cleaning up the tests. Certainly
they could be
>>> made
>>>>>>> even
>>>>>>>>>> better & made to test more things, but at least
they pass now :)
>>>>>>>>>> Much of the change was deleting copy & paste,
and deleting
>>> commented
>>>>>>> out
>>>>>>>>>> tests:
>>>>>>>>>> 53 files changed, 941 insertions(+), 2610 deletions(-)
>> ---------------------------------------------------------------------
>>>>>>>>>> To unsubscribe, e-mail:
>>>>>>>>>> For additional commands, e-mail:
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail:
>>>> For additional commands, e-mail:

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message