[QGIS-Developer] QGIS repository management

[Sandro Santilli]

Hi Alessandro, replies inline

On Sat, Oct 14, 2023 at 11:17:27AM +0200, Alessandro Pasotti wrote:

> I understand your frustration, CI is often failing for no reason and
> we are wasting a lot of time (and CPU power) to re-run the failing
> workflows, any effort to fix these issues is greatly appreciated.

Indeed. It's not always possible to fix them though,
as sometimes they just do not depend on us. For example:
https://github.com/qgis/QGIS/actions/runs/6509998580/job/17687048243?pr=54923#step:14:12

> Coming to your questions, AFAIK whoever has commit rights can approve
> someone else's PR but cannot self approve

Do you see how this makes PR coming from non-committers easier to
merge ? They basically require 1 committer approval, whereas PR
coming from committers require 2 committer approval (the proponent
and another reviewer).

Beside, is there an official place from which we can read the list
of people with "commit rights" ? The term is not in the name or
description of any of the github teams:
https://github.com/orgs/qgis/teams

That term "committer", btw, should probably be changed nowadays as
with git it doesnt' make much sense.

> The QGIS organization has a budget for PR queue management (triage)
> and a separate entry for reviews (budget is public
> https://www.qgis.org/en/site/getinvolved/governance/finance/index.html)
> , in the past this has been working well while I agree that recently
> PRs have been pending without review for a (too) long time.

The bugfixing spreadsheet has a column to report reviewer names,
but it looks like I'm the only one filling it at the moment.
You may notice I've also reported reviews from non-committers
( Laurențiu Nicola, in Cc ).

Is there another document (or git commandline) that can be used to
track how the review budget is being spent ?

> If I am not mistaken, everyone with commit rights can still push to
> master directly without opening a PR

Unfortunately this is not the case:

	[strk at c19:/usr/local/src/qgis/qgis/src/master(master)] git push
	...
	remote: error: GH006: Protected branch update failed for refs/heads/master.
	remote: error: Changes must be made through a pull request. 20 of 20 required status checks are expected.
	To github.com:qgis/QGIS.git
	 ! [remote rejected]         master -> master (protected branch hook declined)
	error: failed to push some refs to 'github.com:qgis/QGIS.git'

> , this is not generally
> recommended (I think I have been doing this only once or twice in the
> last few years), however a direct push to master is acceptable for a
> few cases (e.g. to fix an one-liner change issue where the developer
> is fixing his/hers own mess and is 100% sure there will not be side
> effects, fix a typo in a comment, for last minute fixes done by the
> package maintainers when making the releases etc.).

I agree about the usefulness of both reviews and CI in some cases.
For GEOS and PostGIS, where we do NOT have enforcement of this rule,
we all very often use branches or pull requests or merge requests for
the purpose of getting CI and/or peer reviews on our code. It's the
enforcement of the rule that's blocking.

> A small group of us (3-4 developers including me) have admin access to
> all QGIS repositories and we can bypass any check and merge all PRs
> without approval.

(where) is the composition of this group visible ? Are there documented
procedures describing how a "committer" can be added to or removed from
this group ?

> I admit that I have been using these superpowers more and more often
> (IIRC three or four times) in the last year while I have never felt
> the need to use them before, in an ideal world it should not be
> necessary except for the same use cases I have pointed out for the
> direct pushes to master.

The world is not ideal. In my ideal world we would not even need to
write software, let alone fix bugs !  :P

> The bottom line is that the situation is not perfect and can certainly
> be improved but at the end of the day we are all doing our best to
> keep the process running as smoothly as we can.

Be assured I trust the QGIS developers and PSC members to all have good
intentions and hope this mail of mine is taken for what it is: a genuine
attempt at finding _toghether_ a solution to a problem I see with the
currently implemented approval mechanism (unbalanced, in my opinion,
toward non-committers).

--strk;