It would be nice to have something that makes sure every patch/PR builds correctly and no unit tests fail. That would save the reviewer some time when that happens but that tends to be a rare case. Even with that, I would either have to build locally or download the build artifact to test the fix, so I'm not sure there's much to gain there. I think our PR-to-build-failure rate is < 5% but I don't have the numbers to back that (yet ;) ).
On Fri, Jun 22, 2012 at 12:48 PM, Lance Speelmon <[email protected]> wrote: > Good points - I certainly do not want to dilute the code review practices > already in place. Just trying to reduce the amount of time it takes to do > so… Any ideas on how to get the best of both worlds? i.e. letting CI do > what it is good at but not compromising code review? > > > On Jun 22, 2012, at 11:26 AM, Chris Tweney <[email protected]> > wrote: > > > For the most part, I think committers need to build every PR they > > review, for a couple of reasons: > > > > 1) Smoke testing and edge case testing is part of the review process. If > > you don't build it, you obviously aren't running it and making sure it > > works. Code reviews that only involve reading code tend to drift toward > > mere enforcement of style standards, which is necessary, but not > > sufficient. We adopted code reviews because there was a time when a lot > > of unfinished, untested work was getting committed. > > > > 2) We have a lot of commits going in, and most builds have multiple > > commits. If a build breaks, everyone who committed in that build needs > > to stop their work and check the build. And everyone unlucky enough to > > pull from master during the broken build time will have their work > > interrupted. That amounts to more people and more time lost than if the > > committer runs a build. > > > > Now, there are occasional exceptions -- I just merged a pull from Zach > > that I didn't build. But it was a 2-line change that just introduced the > > use of a constant in place of a numeric literal. Not the kind of thing > > that really needed a second compile. (Of course, if the constant was > > misspelled, I'll have egg on my face in a little while... but then, so > > will Zach, for submitted an uncompiled change!) > > > > One thing that would save a lot of my time is if I had a second box to > > do builds on. I should look into getting an EC2 instance to do builds > > on. I think my usage might fit into the free usage tier. > > > > -chris > > > > > > On 6/22/12 9:39 AM, Lance Speelmon wrote: > >> Is it correct in assuming we have CI builds for every commit in > projects? If so, could I suggest that people's time is too valuable to > spend building every PR manually? > >> > >> WDYT? Thanks, L > >> _______________________________________________ > >> oae-dev mailing list > >> [email protected] > >> http://collab.sakaiproject.org/mailman/listinfo/oae-dev > > _______________________________________________ > > oae-dev mailing list > > [email protected] > > http://collab.sakaiproject.org/mailman/listinfo/oae-dev > > _______________________________________________ > oae-dev mailing list > [email protected] > http://collab.sakaiproject.org/mailman/listinfo/oae-dev >
_______________________________________________ oae-dev mailing list [email protected] http://collab.sakaiproject.org/mailman/listinfo/oae-dev
