<div dir="ltr">Hi Nyall,<div class="gmail_extra">
<br><div class="gmail_quote">On Wed, Oct 14, 2015 at 1:59 PM, Nyall Dawson <span dir="ltr"><<a href="mailto:nyall.dawson@gmail.com" target="_blank">nyall.dawson@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">On 14 October 2015 at 22:31, Larry Shaffer <<a href="mailto:larrys@dakotacarto.com">larrys@dakotacarto.com</a>> wrote:<br>
> Hi,<br>
><br>
> Seems like all protections (against force push, deleting and failing tests)<br>
> are a good fit.<br>
><br>
> There may be instances where some tests are failing on Mac, due to an oddity<br>
> with that platform and not with the actual code per se, i.e. the test needs<br>
> updated for Mac or some Mac-specific bit needs added to source. In light of<br>
> such cases, it might be best to not require that all tests pass for the<br>
> Merge button to be enabled, or it may force some devs to wait until<br>
> Mac-specific issues are addressed first.<br>
><br>
> If we if strictly enforce all tests pass, maybe there is a way to tell<br>
> Travis that a failure on one of the test platforms is non-critical (Mac),<br>
> but a failure on another (Linux) is. ??<br>
<br>
</span>Yes, we can do that. Edit ci/travis/osx/script.sh and modify the<br>
exceptions line (the -E regex) to include the test. Then it'll be<br>
excluded on just OSX but not Linux. You'll see the PyQgsServer test is<br>
already excluded for those very reasons. There's also two tests which<br>
are excluded on every platform (OSM and WCS), as they rely on<br>
unreliable 3rd party services and were occasionally failing for<br>
reasons outside of our control.<br></blockquote><div><br></div><div>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.</div><div><br></div><div>Then, I think that protection should be turned on, since both workarounds are at the reviewer's discretion.</div><div><br></div><div>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.</div><div><br></div><div>[0] <a href="http://dash.orfeo-toolbox.org/testDetails.php?test=33712629&build=203146">http://dash.orfeo-toolbox.org/testDetails.php?test=33712629&build=203146</a></div><div><br></div><div>Regards,</div><div><br></div>Larry Shaffer<br>Dakota Cartography<br><div>Black Hills, South Dakota</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Note that failing test protection doesn't block developers with commit<br>
rights from pushing code which fails tests (which I think is a good<br>
thing), it just blocks the merge button on the github page. This means<br>
the reviewer can still manually merge and push the PR if there's a<br>
good reason to allow a failing test in.<br>
<br>
Tim can you switch this on our Github repo?<br>
<span class=""><font color="#888888"><br>
Nyall<br>
</font></span><div class=""><div class="h5"><br>
<br>
<br>
<br>
<br>
<br>
<br>
><br>
> Regards,<br>
><br>
> Larry Shaffer<br>
> Dakota Cartography<br>
> Black Hills, South Dakota<br>
><br>
> On Tue, Oct 13, 2015 at 4:41 PM, Nathan Woodrow <<a href="mailto:madmanwoo@gmail.com">madmanwoo@gmail.com</a>> wrote:<br>
>><br>
>> +1 not allowing force push is a good thing IMO<br>
>><br>
>> On Wed, Oct 14, 2015 at 8:39 AM, Nyall Dawson <<a href="mailto:nyall.dawson@gmail.com">nyall.dawson@gmail.com</a>><br>
>> wrote:<br>
>>><br>
>>> This seems like a really good idea -<br>
>>><br>
>>> <a href="https://github.com/blog/2051-protected-branches-and-required-status-checks" rel="noreferrer" target="_blank">https://github.com/blog/2051-protected-branches-and-required-status-checks</a><br>
>>><br>
>>> I noticed it's now been rolled out, and think we should enable this on<br>
>>> the QGIS repo ASAP. I can't see any downsides to enabling this and<br>
>>> lots of upsides. Can someone with rights do this?<br>
>>><br>
>>> Nyall<br>
>>> _______________________________________________<br>
>>> Qgis-developer mailing list<br>
>>> <a href="mailto:Qgis-developer@lists.osgeo.org">Qgis-developer@lists.osgeo.org</a><br>
>>> <a href="http://lists.osgeo.org/mailman/listinfo/qgis-developer" rel="noreferrer" target="_blank">http://lists.osgeo.org/mailman/listinfo/qgis-developer</a><br>
>><br>
>><br>
>><br>
>> _______________________________________________<br>
>> Qgis-developer mailing list<br>
>> <a href="mailto:Qgis-developer@lists.osgeo.org">Qgis-developer@lists.osgeo.org</a><br>
>> <a href="http://lists.osgeo.org/mailman/listinfo/qgis-developer" rel="noreferrer" target="_blank">http://lists.osgeo.org/mailman/listinfo/qgis-developer</a><br>
><br>
><br>
><br>
> _______________________________________________<br>
> Qgis-developer mailing list<br>
> <a href="mailto:Qgis-developer@lists.osgeo.org">Qgis-developer@lists.osgeo.org</a><br>
> <a href="http://lists.osgeo.org/mailman/listinfo/qgis-developer" rel="noreferrer" target="_blank">http://lists.osgeo.org/mailman/listinfo/qgis-developer</a><br>
</div></div></blockquote></div><br></div></div>