On Fri, 8 Nov 2019 at 19:04, Denis Rouzaud <[email protected]> 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 > [email protected] > https://lists.osgeo.org/mailman/listinfo/qgis-psc _______________________________________________ QGIS-Developer mailing list [email protected] List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
