[Qgis-psc] [QGIS-Developer] Direct push forbidden to master
Matthias Kuhn
matthias at opengis.ch
Sun Nov 10 03:49:17 PST 2019
Hi Alessandro
On 11/10/19 9:20 AM, Alessandro Pasotti wrote:
>
> On Sat, Nov 9, 2019 at 2:48 PM Matthias Kuhn <matthias at opengis.ch
> <mailto: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.
I am sorry to disagree on this first part. I don't think this warrants a
direct push. I don't even think it saves time. I usually don't have a
clean master checked out in this scenario, so I still need to add a
separate worktree where I can cherry-pick something and then I can as
well just push it to a new PR branch that lets me figure out if this
introduced a new typo or if I forgot to sync the changes to sip.
>
> 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?)
Integrating more static tests in the pre-commit hook sounds like a very
good idea (i.e. the typo checks you mention).
It would also be nice to improve the possibilities to run the CI
containers locally. It will still be tough to debug because it's not
directly on the system but inside a container (i.e. no direct IDE
debugger integration), but I can definitely see some advantages there.
>
> 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
There's some interesting recent information on this too
https://www.qt.io/blog/2019/08/01/precompiled-headers-and-unity-jumbo-builds-in-upcoming-cmake
>
> 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 <mailto: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
>> <mailto: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 <mailto:Qgis-psc at lists.osgeo.org>
>> https://lists.osgeo.org/mailman/listinfo/qgis-psc
>>
>>
>> _______________________________________________
>> Qgis-psc mailing list
>> Qgis-psc at lists.osgeo.org <mailto:Qgis-psc at lists.osgeo.org>
>> https://lists.osgeo.org/mailman/listinfo/qgis-psc
> _______________________________________________
> Qgis-psc mailing list
> Qgis-psc at lists.osgeo.org <mailto:Qgis-psc at lists.osgeo.org>
> https://lists.osgeo.org/mailman/listinfo/qgis-psc
>
>
>
> --
> Alessandro Pasotti
> w3: www.itopen.it <http://www.itopen.it>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/qgis-psc/attachments/20191110/f348a19b/attachment.html>
More information about the Qgis-psc
mailing list