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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Nyall Dawson
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.

Right now there's like... 2... of us doing this on a daily basis, and
we need more hands and eyes. Regressions are fixed by backports, but
they can also be introduced by backports. If we don't get more people
involved in this part of QGIS' maintenance then the stability of the
patch releases is compromised.

While I'm more than happy to keep reviewing backports which touch
areas of code I know well, we have a whole stack of backports for
server fixes, relation fixes, and other parts of QGIS I don't know
well enough to risk release stability by reviewing myself. These
backports are currently sitting in the queue for weeks without review,
and often they stale out and are closed without ever getting merged.

I've also a ton of backports for fixes that I make on a daily basis
which I cannot self review, so the PR queue gets flooded with my
backports until I actively pester/harass others to review and approve.

The truth is that the longer the PR queue, the more it stresses me out
and the less motivation I have to review the incoming feature PRs. We
have many older feature PRs which are stalled and waiting review for
months or longer, just because I'm so flooded with managing newer
incoming PRs that I never have time to review the older ones...

So please, if you know parts of the QGIS code well, and can spare 15
minutes a day to looking over the backport queue and approving any
backports you are confident are regression free, then please get
involved! (or ask your employer to officially allocate you this 15
minutes/day as a way of them supporting QGIS!)

Nyall
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: A plea: more volunteers needed for reviewing backports

Alexis R.L.
Greetings,

I'm curious to know if all backport should be reviewed. In theory other than major code rework backports should be retrocompatible. Assuming they are most often done by domain experts as is mostly the case, would it be too risky to merge if the main PR was reviewed and the tests are all green?

I'd agree there should be exceptions such as after major refactoring (if exclusive to some versions).

Or could there be some way to simply identify backport that would require review and those that would not affect more than the intended area? I'm also curious to know if there could be a way to rely less on the few highly-skilled contributors that are active when it comes to more minor things (if there are such a thing).

Thanks as usual and have a great day,

Alex


Le lun. 22 mars 2021 à 19:07, Nyall Dawson <[hidden email]> a écrit :
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.

Right now there's like... 2... of us doing this on a daily basis, and
we need more hands and eyes. Regressions are fixed by backports, but
they can also be introduced by backports. If we don't get more people
involved in this part of QGIS' maintenance then the stability of the
patch releases is compromised.

While I'm more than happy to keep reviewing backports which touch
areas of code I know well, we have a whole stack of backports for
server fixes, relation fixes, and other parts of QGIS I don't know
well enough to risk release stability by reviewing myself. These
backports are currently sitting in the queue for weeks without review,
and often they stale out and are closed without ever getting merged.

I've also a ton of backports for fixes that I make on a daily basis
which I cannot self review, so the PR queue gets flooded with my
backports until I actively pester/harass others to review and approve.

The truth is that the longer the PR queue, the more it stresses me out
and the less motivation I have to review the incoming feature PRs. We
have many older feature PRs which are stalled and waiting review for
months or longer, just because I'm so flooded with managing newer
incoming PRs that I never have time to review the older ones...

So please, if you know parts of the QGIS code well, and can spare 15
minutes a day to looking over the backport queue and approving any
backports you are confident are regression free, then please get
involved! (or ask your employer to officially allocate you this 15
minutes/day as a way of them supporting QGIS!)

Nyall
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer

_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: A plea: more volunteers needed for reviewing backports

Andreas Neumann-3
In reply to this post by Nyall Dawson
Hi Nyall,

Thanks for raising this issue!

Back in the "old days" we had assigned "maintainers" to different areas of the code base. Maybe we could revive this idea but rename it to "reviewers" and ideally assign more than one person to each area of expertise? It could be published again, somewhere in the governance section of our website, e.g. at https://www.qgis.org/en/site/getinvolved/governance/governance.html

As a side effect, if we had such a list, a potential customer could also see - oh this developer is a reviewer/maintainer of this particular topic - it could be an incentive to pick this developer or company for development work. For some insiders, it is a well-known which dev/company has expertise in which area of QGIS, but for non-insiders, this is not really obvious.

Ideally, besides the volunteers, each of the companies that is active in QGIS development (like Lutra, Oslandia, OPENGIS, 3Liz, etc. - just to name a few) could offer one person as a reviewer to a certain area in the code base. Of course, some, such as Norbit and Lutra are already really active in packaging QGIS ... and North Road and OPENGIS is already quite active in reviewing and other maintenance work.

Greetings,
Andreas

On Tue, 23 Mar 2021 at 00:06, Nyall Dawson <[hidden email]> 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.

Right now there's like... 2... of us doing this on a daily basis, and
we need more hands and eyes. Regressions are fixed by backports, but
they can also be introduced by backports. If we don't get more people
involved in this part of QGIS' maintenance then the stability of the
patch releases is compromised.

While I'm more than happy to keep reviewing backports which touch
areas of code I know well, we have a whole stack of backports for
server fixes, relation fixes, and other parts of QGIS I don't know
well enough to risk release stability by reviewing myself. These
backports are currently sitting in the queue for weeks without review,
and often they stale out and are closed without ever getting merged.

I've also a ton of backports for fixes that I make on a daily basis
which I cannot self review, so the PR queue gets flooded with my
backports until I actively pester/harass others to review and approve.

The truth is that the longer the PR queue, the more it stresses me out
and the less motivation I have to review the incoming feature PRs. We
have many older feature PRs which are stalled and waiting review for
months or longer, just because I'm so flooded with managing newer
incoming PRs that I never have time to review the older ones...

So please, if you know parts of the QGIS code well, and can spare 15
minutes a day to looking over the backport queue and approving any
backports you are confident are regression free, then please get
involved! (or ask your employer to officially allocate you this 15
minutes/day as a way of them supporting QGIS!)

Nyall
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer


--

--
Andreas Neumann
QGIS.ORG board member (treasurer)

_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: A plea: more volunteers needed for reviewing backports

Nyall Dawson
In reply to this post by Nyall Dawson
On Tue, 23 Mar 2021 at 09:06, Nyall Dawson <[hidden email]> 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
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: A plea: more volunteers needed for reviewing backports

pcav
Hi Nyall,

Il 08/04/21 00:28, Nyall Dawson ha scritto:

> 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.

as a PSC we have talked about this.
With my business hat on, I can assure you we have not been deaf to your
call. I have thought about how to contribute to solve this, bt I did not
find a practical way to do it. The main point (clearly outlined by
Alessandro Pasotti during our latest PSC) is that very few people are
technically able to review the PRs, and usually only for the part of
code they already know well. I think this explains the lack of response.
To improve the situation I think we should provide an easy way to
contribute, in a way that actually favours those who contribute.
All the best.
--
Paolo Cavallini
www.faunalia.eu - QGIS.org
training, support, development on QGIS, PostGIS and more
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: A plea: more volunteers needed for reviewing backports

jcabieces
In reply to this post by Nyall Dawson

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, but it's very little compared to the QGIS code base.

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.

Kind regard,
Julien

> On Tue, 23 Mar 2021 at 09:06, Nyall Dawson <[hidden email]> 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
> [hidden email]
> List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
> Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer

_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: A plea: more volunteers needed for reviewing backports

Nyall Dawson
On Thu, 8 Apr 2021 at 18:26, Julien Cabieces
<[hidden email]> 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 <[hidden email]> 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
> > [hidden email]
> > List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
> > Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
>
> _______________________________________________
> QGIS-Developer mailing list
> [hidden email]
> List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
> Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: A plea: more volunteers needed for reviewing backports

Peter Petrik
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 <[hidden email]> wrote:
On Thu, 8 Apr 2021 at 18:26, Julien Cabieces
<[hidden email]> 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 <[hidden email]> 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
> > [hidden email]
> > List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
> > Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
>
> _______________________________________________
> QGIS-Developer mailing list
> [hidden email]
> List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
> Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer

_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: A plea: more volunteers needed for reviewing backports

Matthias Kuhn 🌍
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 <[hidden email]> 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 <[hidden email]> wrote:
On Thu, 8 Apr 2021 at 18:26, Julien Cabieces
<[hidden email]> 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 <[hidden email]> 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
> > [hidden email]
> > List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
> > Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
>
> _______________________________________________
> QGIS-Developer mailing list
> [hidden email]
> List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
> Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer

_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: A plea: more volunteers needed for reviewing backports

Giovanni Manghi
In reply to this post by Nyall Dawson
Hi all,


> 2.  we may have a (partially paid?) role for the review manager, to go
> through the PR queue, ping people for reviews, etc.

Has this been considered/discussed (having 1 or more paid reviewers)?

At very least this seems a reasonable solution until we are not able
to bring in more reviewers.

cheers

-- G --
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: A plea: more volunteers needed for reviewing backports

Alexis R.L.
I agree with Gio and I was curious as to why some bugs are funded but not reviewing. Reviewing can help prevent some bugs and has also the potential to improve a PR. 

I'd also say that improving the review pace will help losing PR to stalebot. In my eyes bugfix are crucial, but new features are also important, and those seem to require more time to review. New features also help promote QGIS and compete against other software.

Funding could be done like bugs or like grants, with or without a selection process. This would help competent devs to devote more of their time to reviewing.

Alex


Le ven. 9 avr. 2021 à 12:42, Giovanni Manghi <[hidden email]> a écrit :
Hi all,


> 2.  we may have a (partially paid?) role for the review manager, to go
> through the PR queue, ping people for reviews, etc.

Has this been considered/discussed (having 1 or more paid reviewers)?

At very least this seems a reasonable solution until we are not able
to bring in more reviewers.

cheers

-- G --
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer

_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: A plea: more volunteers needed for reviewing backports

Alessandro Pasotti-2
On Fri, Apr 9, 2021 at 11:11 PM Alexis R.L. <[hidden email]> wrote:

>
> I agree with Gio and I was curious as to why some bugs are funded but not reviewing. Reviewing can help prevent some bugs and has also the potential to improve a PR.
>
> I'd also say that improving the review pace will help losing PR to stalebot. In my eyes bugfix are crucial, but new features are also important, and those seem to require more time to review. New features also help promote QGIS and compete against other software.
>
> Funding could be done like bugs or like grants, with or without a selection process. This would help competent devs to devote more of their time to reviewing.
>
> Alex
>
>
> Le ven. 9 avr. 2021 à 12:42, Giovanni Manghi <[hidden email]> a écrit :
>>
>> Hi all,
>>
>>
>> > 2.  we may have a (partially paid?) role for the review manager, to go
>> > through the PR queue, ping people for reviews, etc.
>>
>> Has this been considered/discussed (having 1 or more paid reviewers)?
>>
>> At very least this seems a reasonable solution until we are not able
>> to bring in more reviewers.
>>


Hi,

this was discussed a few times, just to clarify where we stand now:

- during the last few year there has been a small QGIS.org budget for
PR queue management which includes reviews to some extent, see the
budget documents and financial reports
https://www.qgis.org/en/site/getinvolved/governance/finance/index.html?highlight=finances,
Matthias, Nyall and (recently, for a small part) myself have been the
recipients of the funds. I can only speak for myself but I'm pretty
sure that this stands also for Nyall and Matthias when I say that
these funds only covered a tiny fraction of the time that we spend on
PR reviews.
- most core developers that I personally know, regularly allocate
budget for doing PR reviews when they do paid work, this budget is
used for PR reviews by other QGIS core committers of the same company
(if your company is so lucky to have more than one) or on an exchange
basis, for example Nyall asks me to review his PR and I ask him to do
the same for me (which happens more often than the opposite): this
happens all the times.
- when we (core developers) work on the QGIS.org paid bugfixing
program before the releases we switch to full "QGIS dev" mode and we
also do PR reviews because it's just part of the process.

Now, my personal opinion about where the problems are and how to
possibly solve them, I'm talking about the most complex PRs, obviously
there are PRs that are really trivial and quick to review:

- PR reviews may be very hard and time consuming and are an assumption
of responsibility, I don't think that there is any single developer
who feels comfortable to do reviews on the entire (huge) QGIS code
base.
- As already mentioned by Matthias, there are blurry lines between
what is acceptable for backporting, the policy has changed a few times
and it is not yet entirely clear to me, this can slow down the PR
review process on backports.
- PR reviews require a deep understanding of the QGIS internals plus a
knowledge of the history of the project and why things have been
implemented in a certain way (that doesn't obviously mean that they
couldn't be refactored).
- The entry barriers for QGIS developments are very high, we all know
that, the code base is huge, the different technologies you need to
know are many and the QA process (CI, tests, code style, spelling,
name your favourite blocker here) is terribly complex (with a reason
because it ultimately leads to better and more robust code!).

That said, I feel that we are now in a better situation than we were a
couple of years ago when the PR queue was much longer compared to now.

But how can we speed up the process?

- the obvious solution is to increase the budget for PR reviewers and
enlarge the number of developers involved to make sure we can cover a
larger area of the code base. Ideally we should be able to pay for a
regular time slot of each involved developer (say, just for example: 4
hours a week?), this may not be evenly distributed, for example a
developer that takes care of the server PRs might require a smaller
time slot than one that works on 3D (just an example, really).
- have clear rules about what can or cannot be backported and when (do
we skip a point release now?, for LTR only?)
- all developers that are working on a funded feature/bugfix/whatever
**MUST** allocate budget for PR reviews (including backports) and take
care of that by paying another developer (if possible from a different
company).
- bring in more developers!

The last one is clearly very difficult and probably worth its own
thread, also because it cannot happen overnight.

What I am 100% sure about is that we cannot just hire some random
developer that knows little about QGIS development to do the work for
us, the burden of PR reviews must stay on the QGIS core developers
team, we must find a way to allow the core developers to **regularly**
spend more time on that activity.

And last but not least: time is a limiting resource and we all do have a life :)

--
Alessandro Pasotti
QCooperative:  www.qcooperative.net
ItOpen:   www.itopen.it
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: A plea: more volunteers needed for reviewing backports

Nyall Dawson
In reply to this post by Nyall Dawson
On Thu, 8 Apr 2021 at 08:28, Nyall Dawson <[hidden email]> wrote:

>
> On Tue, 23 Mar 2021 at 09:06, Nyall Dawson <[hidden email]> 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.
>

Hi all -- just wanted to send out a quick "thanks" to everyone who has
replied and the valuable discussion this has kicked off. I'm going
(mostly) off grid for the next week (camping) so my replies to
incoming emails will be delayed, but I promise to reply to all the
questions sent my way when I'm back :)

Nyall
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: A plea: more volunteers needed for reviewing backports

Andreas Neumann-3
In reply to this post by Alessandro Pasotti-2
Hi Alex, Hi all,

Thank you for this very good summary, which summarizes the problems and potential solutions pretty well. On the financial side I can say that for 2021 we increased the budget available for Github and Travis management, and PR reviewing from 6k in 2020 to 10k in 2021 - related projects like the Travis to Github CI migration are not included here and have their separate budget item.

However, as Alessandro pointed out, the main issue here is finding skilled persons who know the QGIS project and code base good enough to be able to help in the reviewing process.

Nevertheless, I will try to increase the budget item dedicated to CI maintenance and PR reviewing again in 2022. Hopefully, in parallel, the reviewing process and rules can be better formalized and more people can join the effort.

Greetings,
Andreas



On Sat, 10 Apr 2021 at 10:44, Alessandro Pasotti <[hidden email]> wrote:
On Fri, Apr 9, 2021 at 11:11 PM Alexis R.L. <[hidden email]> wrote:
>
> I agree with Gio and I was curious as to why some bugs are funded but not reviewing. Reviewing can help prevent some bugs and has also the potential to improve a PR.
>
> I'd also say that improving the review pace will help losing PR to stalebot. In my eyes bugfix are crucial, but new features are also important, and those seem to require more time to review. New features also help promote QGIS and compete against other software.
>
> Funding could be done like bugs or like grants, with or without a selection process. This would help competent devs to devote more of their time to reviewing.
>
> Alex
>
>
> Le ven. 9 avr. 2021 à 12:42, Giovanni Manghi <[hidden email]> a écrit :
>>
>> Hi all,
>>
>>
>> > 2.  we may have a (partially paid?) role for the review manager, to go
>> > through the PR queue, ping people for reviews, etc.
>>
>> Has this been considered/discussed (having 1 or more paid reviewers)?
>>
>> At very least this seems a reasonable solution until we are not able
>> to bring in more reviewers.
>>


Hi,

this was discussed a few times, just to clarify where we stand now:

- during the last few year there has been a small QGIS.org budget for
PR queue management which includes reviews to some extent, see the
budget documents and financial reports
https://www.qgis.org/en/site/getinvolved/governance/finance/index.html?highlight=finances,
Matthias, Nyall and (recently, for a small part) myself have been the
recipients of the funds. I can only speak for myself but I'm pretty
sure that this stands also for Nyall and Matthias when I say that
these funds only covered a tiny fraction of the time that we spend on
PR reviews.
- most core developers that I personally know, regularly allocate
budget for doing PR reviews when they do paid work, this budget is
used for PR reviews by other QGIS core committers of the same company
(if your company is so lucky to have more than one) or on an exchange
basis, for example Nyall asks me to review his PR and I ask him to do
the same for me (which happens more often than the opposite): this
happens all the times.
- when we (core developers) work on the QGIS.org paid bugfixing
program before the releases we switch to full "QGIS dev" mode and we
also do PR reviews because it's just part of the process.

Now, my personal opinion about where the problems are and how to
possibly solve them, I'm talking about the most complex PRs, obviously
there are PRs that are really trivial and quick to review:

- PR reviews may be very hard and time consuming and are an assumption
of responsibility, I don't think that there is any single developer
who feels comfortable to do reviews on the entire (huge) QGIS code
base.
- As already mentioned by Matthias, there are blurry lines between
what is acceptable for backporting, the policy has changed a few times
and it is not yet entirely clear to me, this can slow down the PR
review process on backports.
- PR reviews require a deep understanding of the QGIS internals plus a
knowledge of the history of the project and why things have been
implemented in a certain way (that doesn't obviously mean that they
couldn't be refactored).
- The entry barriers for QGIS developments are very high, we all know
that, the code base is huge, the different technologies you need to
know are many and the QA process (CI, tests, code style, spelling,
name your favourite blocker here) is terribly complex (with a reason
because it ultimately leads to better and more robust code!).

That said, I feel that we are now in a better situation than we were a
couple of years ago when the PR queue was much longer compared to now.

But how can we speed up the process?

- the obvious solution is to increase the budget for PR reviewers and
enlarge the number of developers involved to make sure we can cover a
larger area of the code base. Ideally we should be able to pay for a
regular time slot of each involved developer (say, just for example: 4
hours a week?), this may not be evenly distributed, for example a
developer that takes care of the server PRs might require a smaller
time slot than one that works on 3D (just an example, really).
- have clear rules about what can or cannot be backported and when (do
we skip a point release now?, for LTR only?)
- all developers that are working on a funded feature/bugfix/whatever
**MUST** allocate budget for PR reviews (including backports) and take
care of that by paying another developer (if possible from a different
company).
- bring in more developers!

The last one is clearly very difficult and probably worth its own
thread, also because it cannot happen overnight.

What I am 100% sure about is that we cannot just hire some random
developer that knows little about QGIS development to do the work for
us, the burden of PR reviews must stay on the QGIS core developers
team, we must find a way to allow the core developers to **regularly**
spend more time on that activity.

And last but not least: time is a limiting resource and we all do have a life :)

--
Alessandro Pasotti
QCooperative:  www.qcooperative.net
ItOpen:   www.itopen.it
_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer


--

--
Andreas Neumann
QGIS.ORG board member (treasurer)

_______________________________________________
QGIS-Developer mailing list
[hidden email]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer