[QGIS-Developer] A plea: more volunteers needed for reviewing backports

Peter Petrik peter.petrik at lutraconsulting.co.uk
Fri Apr 9 00:21:03 PDT 2021


Hi,

First of all, thanks Nyall for your efforts. I try to review the code that
we introduced to QGIS and I find kind-of familiar: for example QgsQuick,
MeshLayer or MacOS stuff. But as mentioned in the thread, the less you know
the particular code, the more time it takes to review it.  What I see that
I can improve is to actively at least twice per week double check the PR
queue to see if there is something in this area, since sometimes I need to
be pinged to get a notification in email that the PR is waiting for review.

What are my thoughts on other improvements not mentioned by Nyall and
others:
1. when someone introduces a new feature (and most notably coming from a
paid contract), the review process should be taken into account in the
quote and the reviewer should be notified. Proper review can take days...
2.  we may have a (partially paid?) role for the review manager, to go
through the PR queue, ping people for reviews, etc. (Alternatively) create
some github-action bot to assign the maintainers to the PRs as reviewers?
3. reintroduce the maintainers of particular QGIS code-base. For example
here the motivation for companies could be to advertise their logo in the
"sustainable member section"  or introducing some other category to
advertise their efforts.

Cheers,
Peter


On Fri, Apr 9, 2021 at 7:45 AM Nyall Dawson <nyall.dawson at gmail.com> wrote:

> On Thu, 8 Apr 2021 at 18:26, Julien Cabieces
> <julien.cabieces at oslandia.com> wrote:
> >
> >
> > Hi Nyall,
> >
> > We (at Oslandia) heard your call for more volunteers in reviewing the
> backports and PRs.
> >
> > We are very well aware that our contribution effort for this task is not
> enough. I see mainly two reasons:
> > - Lack of time and human ressources to fullfill this task. This is a bad
> reason, and a well known situation that we need to take care of internally,
> to free our schedule.
> > - A very partial knowledge of the code base in which we feel relevant to
> review. I try myself to review all Oracle related PRs, or take a look in
> some part of the code I know enough to make a good review,
>
> It would certainly be remiss of me not to acknowledge your hard work
> in maintaining the Oracle provider over recent months -- it's
> certainly very highly valued!
>
> > but it's very little compared to the QGIS code base.
>
> This is where things get really tricky. We do have a very small pool
> of people with the required expertise to handle the reviewing process,
> and it's a very fine balancing act of "opening this up to a wider
> community" vs "ensure that nothing nasty gets through". I don't have
> any easy answers here, but a few random thoughts:
>
> - ANY review is appreciated, even if it's just a "I had a look at this
> and the c++ code looks good to me, but I'm not familiar with the QGIS
> raster api so can someone else check this part out".
> - There's a lot of "chore" work in the pr review process which has a
> lower barrier of entry. For instance guiding new contributors through
> the QGIS processes such as the formatting scripts and CI setup. We
> regularly get PRs from first-time contributors and they almost always
> need some mentoring in the QGIS processes before they pass the CI
> setup and are ready for an in-depth review. This is the kind of thing
> which we should ideally do ASAP, as the faster we get the contributors
> through the initial maze of QGIS contributions the more likely they
> are to continue to contribute.
> - We've discussed this before, but a quick comment from a
> non-developer on new contributors PRs to say "great work, welcome
> abroad, this sounds like a very valuable contribution" is enough to
> give the project a positive vibe and give motivation to the submitter
> to get their PR in shape to merge
> - PR reviewing is a great way to learn new parts of the QGIS codebase ;)
>
> > I hesitated answering your first email, saying basically that we will
> try to improve the situation, but it would have been just a promise, and
> I'm not sure that we would have been able to keep it, regarding the points
> I described above.
>
> I really appreciate the reply, and the time you've taken to drop your
> thoughts into this conversation!
>
> Nyall
>
>
>
>
> >
> >
> > Kind regard,
> > Julien
> >
> > > On Tue, 23 Mar 2021 at 09:06, Nyall Dawson <nyall.dawson at gmail.com>
> wrote:
> > >>
> > >> Hi list,
> > >>
> > >> This is a public plea for more developers who are very familiar with
> > >> different parts of the QGIS codebase to become actively involved in
> > >> backport PR management.
> > >>
> > >> We NEED more people to be actively (i.e. daily) monitoring for
> > >> backport PRs and then reviewing them if the backport affects code
> > >> areas which they're knowledgeable about.
> > >>
> > >
> > > Well my earlier plea seems like it fell on deaf ears, and we've seen
> > > only minimal assistance coming to maintain the PR queue since I posted
> > > this.
> > >
> > > I'll ask nicely once more. If you're an organisation involved in QGIS
> > > development, you NEED to donate time to maintaining this list. It's
> > > not enough to just ensure your own PRs get through the queue, you HAVE
> > > to help with the shared burden of getting others PRs reviewed and
> > > merged.
> > >
> > > This callout applies to ALL organisations who are making money from
> > > core QGIS development. You know who you are, and you can ALL afford to
> > > donate 30 minutes of a staff member's time each week to helping with
> > > this shared burden. If your boss is blocking this then you NEED to let
> > > them know how big a risk they are running here. It's not fair to leave
> > > this responsibility on the already burdened shoulders of a few
> > > overworked individuals.
> > >
> > > Nyall
> > > _______________________________________________
> > > 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
> >
> > _______________________________________________
> > 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
> _______________________________________________
> 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/20210409/ac98b120/attachment.html>


More information about the QGIS-Developer mailing list