[Qgis-psc] [QGIS-Developer] Direct push forbidden to master

Sun Nov 10 00:20:11 PST 2019

On Sat, Nov 9, 2019 at 2:48 PM Matthias Kuhn <matthias at opengis.ch> wrote:

> Hi,
>
> I think this discussion was triggered partially at least by my refactoring
> of the cmake build system. I am sorry for any inconveniences that this
> triggered and hope it's justified by the long term simplification that
> should come out of this.
>
> Overall I see three different topics. One is the request for a Windows
> based CI, one is a rant over travis and CI in general and one is the
> question if direct pushes to master should be allowed (and encouraged).
>
> A Windows based CI would in my opinion be highly desirable. I cannot see
> any reason not to do it actually. Most users run QGIS on Windows, that's
> just the reality. And we want QGIS to run stable on Windows. The questions
> that remain is how much $$ the infrastructure and it's setup is and if we
> get a Windows build system to complete in a reasonable time. I don't think
> nightly builds are equivalent, last week they were broken and it was up to
> one single person (Jürgen) to fix this. If a CI had been in place it would
> have been my job to fix the build before it is merged into our codebase,
> which would have been much more convenient for everyone. We also don't need
> all tests to pass on Windows to put this into play. We can blacklist some
> tests and gradually fix remaining ones. We have done that before on other
> platforms too.
>
> If Travis (or any CI) is worth the effort is indeed a question that can
> and should be asked. It does require a lot of time to keep it up and
> running. What it provides on the other hand is an early feedback of
> regressions. And early in this case means detecting them before they even
> are in the codebase. This saves valuable time from our testers which can
> instead focus on testing things which have not been detected by the CI. And
> it saves time of core developers who don't need to fix bugs introduced by
> others. It also makes it easier to fix bugs because they are related to a
> particular pull request and context. Yes, travis also bitches around
> sometime and makes us loose time. I can't give a definite answer but in
> comparison with the wild west style before I am in favor of our CI.
>
> Pushing directly to master is not a complete disaster, but I can't see
> many advantages of doing it. So far it's sort of a gentlemen agreement that
> most don't push directly to master. I don't remember the last time I did it
> actually. I am fine with restricting and enforcing this on github but if
> that's not desired by the PSC / Jürgen, I would like to appeal to everyone
> to keep away from doing direct pushes. Waiting an hour for a green tick
> shouldn't be asked too much.
>

Hi Matthias,

I totally agree with your last claim: I think we must all go through PRs
and this is what "normally" everyone does, but I see a very few and limited
cases when a direct commit to master is the right thing to do and I
recommend to leave that door open.

A stupid example is fixing a typo in a comment, to be honest if I need to
branch and create a PR and go through Travis I will just leave the typo (if
that's not part of something I'm still working on). Of course, sticking to
this stupid example, this also means that after fixing this typo in the
comment if master is broken I won't go to sleep until I fixed it.

As I've already mentioned I don't see a problem here, 99.99% of the
workflow goes through Travis and in the few exceptional cases when it
doesn't, if master was broken it was fixed almost immediately, in any event
I don't see a big issue if master is broken for some time provided that who
breaks it also takes the responsibility to fix it quickly.

Speaking of Travis, I've found it a life-saver many times but I also think
that given the amount of time we spent on it we could have rewritten QGIS
in nodejs (because this is the long term plan, isn't it?)  but without a
better alternative we must live with it.

Just one thing I would recommend investing a bit more time and money: to be
able to run a Travis-like test suite locally (I was able to do it but it
costed me a lot of time to set up and to understand how to use it, I
definitely don't use it regularly), here are the reasons:
- branch switch costs time, and I'm often working on several fixes/features
at the same time, waiting for Travis to know that it failed because of
(say) a silly typo in a comment is a huge waste of time
- sometimes being able to debug the Travis locally is the only option (I
know there is  way to ssh into remote Travis but that's a bit complicated
and slow)
- my machine is faster than Travis
- prepare-commit doesn't make all checks (spelling, others?)

Cutting down build times would also help but I don't have any real proposal
here, I've read somewhere that other build systems might be faster and that
pre-compiled header might help but I'm not an expert in this field

So, IMHO long life to PRs and Travis for the time being but we need to give
to the developers the tools to run locally all checks that Travis runs
remotely so that they are reasonably sure ("reasonably" because Travis
random unrelated failures are always around the corner) that the build will
pass and they will be able to debug a Travis failure locally without
waiting for hours.

Cheers

-- Matthias
> On 11/8/19 1:23 PM, Nathan Woodrow wrote:
>
> I agree with Even.  The issue here is the slow CI builds and lack of
> Windows CI builds.
> To make this process smoother and more solid for everyone we need to
> invest in better CI not pretend we can just ignore it for the sake of
> getting a commit in..
>
> - Nathan
>
> On Fri, Nov 8, 2019 at 10:05 PM Even Rouault <even.rouault at spatialys.com>
> wrote:
>
>> On vendredi 8 novembre 2019 19:32:06 CET Nyall Dawson wrote:
>> > On Fri, 8 Nov 2019 at 18:33, ElPaso <elpaso at itopen.it> wrote:
>> > > I'm with Jürgen on this, I think we all are responsible developers
>> > > acting for the good of QGIS and I didn't see any abuse on direct
>> commits
>> > > in the past few years.
>> > >
>> > >
>> > > On the contrary, I think I should have committed to master directly to
>> > > correct a PR of mines that I merged by mistake the day before 3.10
>> > > release (https://github.com/qgis/QGIS/pull/32369) instead of waiting
>> for
>> > > an approval that didn't arrive in time (in this case it wasn't really
>> a
>> > > big problem though).
>> >
>> > I don't think Denis is arguing for forced-reviews of pull requests.
>> > Rather just that all changes GO through pull requests so that we can
>> > be sure they don't break CI.
>>
>> What is obvious here is that a CI for Windows would avoid avoided this
>> case,
>> as the cost of maintaining the Windows build alive mostly relies on a
>> single
>> person currently.
>> As it seems that most QGIS users run Windows, it might be worth for the
>> project to invest into that, both in funding the time of the folks who
>> would
>> set up that and possibly paying for a beefy enough server (potentially
>> from a
>> commercial CI hosting solution) that would sustain the load of pull
>> requests.
>>
>> And paying for Travis extra build could potentially save time for a
>> number of
>> contributors. For the github OSGeo organization, mostly used by PROJ &
>> GDAL,
>> it is between 4000-5000 USD/year for 11 parallel builds (this includes a
>> discount for OSGeo being a non-profit). There might be cheeper
>> alternatives by
>> other competitors.
>>
>> --
>> Spatialys - Geospatial professional services
>> http://www.spatialys.com
>> _______________________________________________
>> Qgis-psc mailing list
>> Qgis-psc at lists.osgeo.org
>> https://lists.osgeo.org/mailman/listinfo/qgis-psc
>
>
> _______________________________________________
> Qgis-psc mailing listQgis-psc at lists.osgeo.orghttps://lists.osgeo.org/mailman/listinfo/qgis-psc
>
> _______________________________________________
> Qgis-psc mailing list
> Qgis-psc at lists.osgeo.org
> https://lists.osgeo.org/mailman/listinfo/qgis-psc

-- 
Alessandro Pasotti
w3:   www.itopen.it
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/qgis-psc/attachments/20191110/1889bd80/attachment.html>