[QGIS-Developer] A plea: more volunteers needed for reviewing backports
Locked
[QGIS-Developer] A plea: more volunteers needed for reviewing backports
 
|
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 |
Re: A plea: more volunteers needed for reviewing backports
|
|
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, _______________________________________________ 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 |
Re: A plea: more volunteers needed for reviewing backports
|
|
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, -- _______________________________________________ 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 |
Re: A plea: more volunteers needed for reviewing backports
|
|
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 |
|
|
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 |
|
|
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 |
Re: A plea: more volunteers needed for reviewing backports
|
|
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 |
Re: A plea: more volunteers needed for reviewing backports
|
|
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 _______________________________________________ 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 |
Re: A plea: more volunteers needed for reviewing backports
|
|
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:
_______________________________________________ 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 |
Re: A plea: more volunteers needed for reviewing backports
|
|
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 |
Re: A plea: more volunteers needed for reviewing backports
|
|
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, _______________________________________________ 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 |
Re: A plea: more volunteers needed for reviewing backports
|
|
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 |
Re: A plea: more volunteers needed for reviewing backports
|
|
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 |
Re: A plea: more volunteers needed for reviewing backports
|
|
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: -- _______________________________________________ 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 |
«
Return to QGIS - Developer
|
1 view|%1 views
| Free forum by Nabble | Edit this page |