[Qgis-developer] Adding branch protection to github?

[Larry Shaffer]

Hi Nyall,

On Wed, Oct 14, 2015 at 1:59 PM, Nyall Dawson <nyall.dawson at gmail.com>
wrote:

> On 14 October 2015 at 22:31, Larry Shaffer <larrys at dakotacarto.com> wrote:
> > Hi,
> >
> > Seems like all protections (against force push, deleting and failing
> tests)
> > are a good fit.
> >
> > There may be instances where some tests are failing on Mac, due to an
> oddity
> > with that platform and not with the actual code per se, i.e. the test
> needs
> > updated for Mac or some Mac-specific bit needs added to source. In light
> of
> > such cases, it might be best to not require that all tests pass for the
> > Merge button to be enabled, or it may force some devs to wait until
> > Mac-specific issues are addressed first.
> >
> > If we if strictly enforce all tests pass, maybe there is a way to tell
> > Travis that a failure on one of the test platforms is non-critical (Mac),
> > but a failure on another (Linux) is. ??
>
> Yes, we can do that. Edit ci/travis/osx/script.sh and modify the
> exceptions line (the -E regex) to include the test. Then it'll be
> excluded on just OSX but not Linux. You'll see the PyQgsServer test is
> already excluded for those very reasons. There's also two tests which
> are excluded on every platform (OSM and WCS), as they rely on
> unreliable 3rd party services and were occasionally failing for
> reasons outside of our control.
>

I meant per platform here, not per test. But I see how the PR submitter
could just augment their PR with a commit to exclude the failing test (if
deemed necessary by the reviewer). Or, the reviewer could just push the PR
from their local repo. Either way, the reviewer would then need to notify
someone about fixing the excluded test if they don't know how.

Then, I think that protection should be turned on, since both workarounds
are at the reviewer's discretion.

Odd about the PyQgsServer test... it passes on my Mac, but is the only test
not passing on the Travis instance [0] hooked up to my forked repo (I have
same OS and Xcode versions). In fact all tests now pass on my local Mac,
which is VERY cool. Thanks for explaining why the other tests are excluded.

[0] http://dash.orfeo-toolbox.org/testDetails.php?test=33712629&build=203146

Regards,

Larry Shaffer
Dakota Cartography
Black Hills, South Dakota


> Note that failing test protection doesn't block developers with commit
> rights from pushing code which fails tests (which I think is a good
> thing), it just blocks the merge button on the github page. This means
> the reviewer can still manually merge and push the PR if there's a
> good reason to allow a failing test in.
>
> Tim can you switch this on our Github repo?
>
> Nyall
>
>
>
>
>
>
>
> >
> > Regards,
> >
> > Larry Shaffer
> > Dakota Cartography
> > Black Hills, South Dakota
> >
> > On Tue, Oct 13, 2015 at 4:41 PM, Nathan Woodrow <madmanwoo at gmail.com>
> wrote:
> >>
> >> +1 not allowing force push is a good thing IMO
> >>
> >> On Wed, Oct 14, 2015 at 8:39 AM, Nyall Dawson <nyall.dawson at gmail.com>
> >> wrote:
> >>>
> >>> This seems like a really good idea -
> >>>
> >>>
> https://github.com/blog/2051-protected-branches-and-required-status-checks
> >>>
> >>> I noticed it's now been rolled out, and think we should enable this on
> >>> the QGIS repo ASAP. I can't see any downsides to enabling this and
> >>> lots of upsides. Can someone with rights do this?
> >>>
> >>> Nyall
> >>> _______________________________________________
> >>> Qgis-developer mailing list
> >>> Qgis-developer at lists.osgeo.org
> >>> http://lists.osgeo.org/mailman/listinfo/qgis-developer
> >>
> >>
> >>
> >> _______________________________________________
> >> Qgis-developer mailing list
> >> Qgis-developer at lists.osgeo.org
> >> http://lists.osgeo.org/mailman/listinfo/qgis-developer
> >
> >
> >
> > _______________________________________________
> > Qgis-developer mailing list
> > Qgis-developer at lists.osgeo.org
> > http://lists.osgeo.org/mailman/listinfo/qgis-developer
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/qgis-developer/attachments/20151014/f99926e6/attachment-0001.html>