cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Grieve <agri...@chromium.org>
Subject Re: Android JUnit Tests Now Pass
Date Thu, 12 Feb 2015 19:47:11 GMT
Awesomesauce. Going to move forward then (with putting back the
accidentally deleted test). If there's other things missed, they can be
brought back as well.

On Thu, Feb 12, 2015 at 12:47 PM, Brian LeRoux <b@brian.io> wrote:

> 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