[QGIS-Developer] QGIS repository management

[Matthias Kuhn]

In my experience, the peer reviews have proven to be an effective tool to
improve the code quality.
I think it can be explained with a four eyes principle. The first two eyes
are involved in writing the code, the second pair of eyes that validates
needs to be a trusted pair.
For a code base of the maturity of QGIS, this seems to me to be justified
and reasonable.

As for who can do a review, a potential conflict of interest has been
raised. This arises most noticeable within a single company, but can also
very well be the case whenever a collaboration occurs between multiple
individuals or if a well-connected client is involved. In my experience, in
most cases people will first ask the person that they trust to be most
qualified to review a specific area of the code, regardless the
organisation they work for. In some cases, this may be your colleagues.
What I and others have done in the past in such cases, is to leave an
additional review window of a couple of days before merging to give
everyone the opportunity to jump in if they wanted to, just to be sure to
play it fair. In my experience, this worked quite nicely, but I am also
open to other approaches.

As for failing tests, I think one possibly low hanging fruit would be to
switch all CI workflows to be based on non-rolling distributions. And we
could potentially also be a bit more strict with disabling flaky tests or
making them optional.

Best wishes
Matthias

On Tue, Oct 17, 2023 at 9:47 AM Sandro Santilli via QGIS-Developer <
qgis-developer at lists.osgeo.org> wrote:

> On Tue, Oct 17, 2023 at 03:08:34PM +1300, Nyall Dawson via QGIS-Developer
> wrote:
> > On Tue, 17 Oct 2023 at 03:41, Sandro Santilli <strk at kbt.io> wrote:
> > >
> > > On Mon, Oct 16, 2023 at 09:59:35AM +1300, Nyall Dawson via
> QGIS-Developer wrote:
> > >
> > > > If you flip the situation, you'll see that yes, you do have trust!
> > > >
> > > > - a complete stranger CANNOT approve their own changes
> > > > - a complete stranger CANNOT approve other stranger's changes
> > > > - a complete stranger CANNOT approve an approved member's changes
> > > >
> > > > vs
> > > >
> > > > - an approved member CANNOT approve their own changes
> > > > - an approved member CAN approve a complete stranger's changes
> > > > - an approved member CAN approve a another approved member's changes
> > >
> > > That's partial trust. I'm trusted to be able to judge someone else's
> > > work but not to judge my own work !
> >
> > Ok, it's partial trust. What's wrong with that?
>
> I feel I'm not explaining myself correctly.
> I do believe in peer review, and I'm happy for others to look at the
> changes I propose. I'd love everyone to look at them!
>
> The word "partial" was not appropriate, probably, it's more about
> "inconsistency".
>
> > In short: I'm more than OK with only partial trusting everyone. We ALL
> > make mistakes, so let's stick with the processes we have which reduce
> > the risk of these mistakes hitting end users..
>
> I know I make mistakes, but I'm able to merge changes proposed by a
> random contributor, where my own review is enough.
>
> I'm FULLY trusted in that case, and this is conflicting with not
> being so when it's me proposing the changes.
>
> Why should I be able to merge in that case then ? Is it because
> 2 people review the code ? Me and the proponent ? Then why the
> random contributor review would not count on MY prs ?
>
> I hope you see the discrepancy here.
>
> --strk;
>
>   Libre GIS consultant/developer
>   https://strk.kbt.io/services.html
> _______________________________________________
> QGIS-Developer mailing list
> QGIS-Developer at lists.osgeo.org
> List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
> Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/qgis-developer/attachments/20231017/a5e1632f/attachment.htm>