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

[Nyall Dawson]

On Fri, 8 Nov 2019 at 19:04, Denis Rouzaud <denis.rouzaud at gmail.com> 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.
>
>
> While not strictly talking about abuse, there has been quite a few direct push commits which broke the CI.
> That generally meant looking at someone else work to fix things, sometimes by several persons at the same time.
> It also means all last PRs jobs that have to be restarted.
> It gives a bad image/impression.
> It's frustrating.
>
> One saves himself about one hour time to see his work integrated in the final branch.
> All the good soldiers in the Travis queue are good for a restart

I'm +1 to forced use of pull requests and no direct pushes to master.
The reality is that breakage of CI today cost me quite a lot of wasted
time, when I had a very narrow window to make some bug fixes and see
that they would pass the CI tests before leaving for FOSS4G Oceania
next week.

My view is that it's just not a fair practice for other community
members -- while it avoids the submitter having to wait for the CI
checks to pass, the cost is the risk of causing large disruption to
many other developer's work.

If we all play nice and follow this practice then everyone is
following the same rules. If we relax it and allow force pushes from
some developers then we have inequality. And if we relax it for
everyone, we go back to the anarchy which development was in the early
2.x days (just thinking back on this gives me nightmares. I honestly
can't believe now looking back that we all (myself included) used to
push 1k+ line feature changes direct to master with zero code review
or CI tests. Eeeeeeeeeeeeeeeeee)

Nyall

>
> In the history of breaking commits, it's always "small and insignificant changes which won't affect the build or the CI". I made a few of these in the past too.
> Just because we don't have docker and clang installed in our brains ;)
>
> What are the valid arguments to allow direct pushes?
> I can see only one: the release process might requires this since it relies on the branches themselves.
> But this was clearly not the case in the last event.
>
>>
>> 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).
>
>
> OK, this might be a valid reason: to fix merge errors.
> But this really sounds like an edge case to me.
> Anyway, this directly fit in the release process window I was mentioning above, which might require a special treatment.
>
> So this could be a proposition.
> Enforce PRs in standard time but allow direct push by a list of safe core-devs during the release process window.
>
> Cheers,
> Denis
> _______________________________________________
> Qgis-psc mailing list
> Qgis-psc at lists.osgeo.org
> https://lists.osgeo.org/mailman/listinfo/qgis-psc