cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian LeRoux...@brian.io>
Subject Re: Android JUnit Tests Now Pass
Date Thu, 12 Feb 2015 17:47:20 GMT
I see no situation where we don't want a feature branch vetted by >1 person
before we land anything on master …short of fixing broken tests.

I assume good faith. Joe: you had a bad day and, I think, you still feel
mistrust after previous commits landing on master stalling out your work
last summer. Lets put that behind us.

Andrew pls fire a ping to the list w/ a PR for anything aiming to live on
Android master until earn Joe's trust back. And lets avoid the editorial
about motivations. We all want the same thing here: work on a kick ass open
source project that makes a difference.


On Thu, Feb 12, 2015 at 9:31 AM, Jesse <purplecabbage@gmail.com> wrote:

> 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 <agrieve@chromium.org> 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 <bowserj@gmail.com> wrote:
> >>
> >> On Thu Feb 12 2015 at 7:44:52 AM Andrew Grieve <agrieve@chromium.org>
> >> 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 <purplecabbage@gmail.com>
> 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 <agrieve@chromium.org>
> >>> wrote:
> >>>>>
> >>>>>> On Wed, Feb 11, 2015 at 5:00 PM, Jesse <purplecabbage@gmail.com>
> >>> 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
> >>>>>> risingj.com
> >>>>>>
> >>>>>>> On Wed, Feb 11, 2015 at 1:53 PM, Joe Bowser <bowserj@gmail.com>
> >>> 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 upon
> >>>>>> yourself
> >>>>>>> to throw away all the JUnit tests that didn't pass, but
that misses
> >>> 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
worse
> >>>>>>> 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 a
> >>>> technical
> >>>>>>> reason for any of it.
> >>>>>>>
> >>>>>>>> On Wed Feb 11 2015 at 1:36:19 PM Joe Bowser <bowserj@gmail.com>
> >>>> 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 in
> >> 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 intent
> >>>>>> behind
> >>>>>>>> the test every time we do a refactor.
> >>>>>>>>
> >>>>>>>> Of course, if we're going to throw away the embedded
WebView case,
> >>>> then
> >>>>>>>> maybe there's not value after all.
> >>>>>>>>
> >>>>>>>> On Wed Feb 11 2015 at 1:12:29 PM Andrew Grieve <
> >>> agrieve@chromium.org>
> >>>>>>>> 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 <
> >>> muratsu@microsoft.com
> >>>>>
> >>>>>>>>> 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. (
> >>>>>>>>>> https://travis-ci.org/apache/cordova-android/builds/50295748)
> >>>>>>>>>>
> >>>>>>>>>> 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: agrieve@google.com [mailto:agrieve@google.com]
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: dev-unsubscribe@cordova.apache.org
> >>>>>>>>>> For additional commands, e-mail: dev-help@cordova.apache.org
> >>>>
> >>>> ---------------------------------------------------------------------
> >>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> >>>> For additional commands, e-mail: dev-help@cordova.apache.org
> >>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> For additional commands, e-mail: dev-help@cordova.apache.org
>
>

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