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 <agri...@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 <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 >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
