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

Fri Apr 9 02:09:50 PDT 2021

Hi,

Thanks for the continued effort in reviewing. It's one of the
not-so-visible-highly-technically-and-socially-qualified-people-needed-but-very-important
aspects of (open source) software development.

I try to go over the PR queue every now and then and merge/comment/review
what's possible, but these days I find less time than in the past. While at
the same time it seems pull request activity increases it seems.
I'll make sure we raise this topic at an upcoming OPENGIS.ch meeting and
encourage people to participate.

Additionally to what has been said already, backports are easy to forget /
ignore by the original author because they have been opened by a bot and as
an author you won't receive notifications by reviewers or the stale bot. I
wonder if we could get the original author more involved if we'd mention
them in the backport comment and increase their responsibility for this
part. This might be worth some automation.

Something else is that for example for me, the process to decide which
backports get into pending backports was not obvious at first (only the LTR
or also LR? At which stage of the LTR? How exactly do they get in there?),
I'm sure for other people there are other parts of the process that are not
immediately clear. I think documenting the review process could help here
(the suggestions written by Nyall above for a lower entry barrier into the
reviewer process would already be worth mentioning).

I hope I'll find more time again for reviews myself in the future.
I actually enjoyed it a lot in the past; it has influenced my development a
lot over the years as it allowed me to discuss concepts and approaches with
experts from all over the globe, more than I had the chance before in the
SMB offices I have been employed before.

Matthias

On Fri, Apr 9, 2021 at 9:21 AM Peter Petrik <
peter.petrik at lutraconsulting.co.uk> wrote:

> 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
>>
> _______________________________________________
> 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/7db1848c/attachment-0001.html>