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 <bows...@gmail.com> wrote: > On Thu Feb 12 2015 at 7:44:52 AM Andrew Grieve <agri...@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 <purplecabb...@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 <agri...@chromium.org> > > wrote: > > > > > > > >> On Wed, Feb 11, 2015 at 5:00 PM, Jesse <purplecabb...@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 <bows...@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 <bows...@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 < > > agri...@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 < > > mura...@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: agri...@google.com [mailto:agri...@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-unsubscr...@cordova.apache.org > > > >>>>>> For additional commands, e-mail: dev-h...@cordova.apache.org > > > >> > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org > > > For additional commands, e-mail: dev-h...@cordova.apache.org > > > > > > > > >
