Please help with pull requests!

[gravitystorm]

We have a huge number of open pull requests, and it's unfortunate that so many of them stay open for so long before being reviewed or merged. I'd like to change this situation.

The main bottleneck is that we have more pull requests being created than the maintainers can review each week. Reviewing PRs is often time consuming, and with the best will in the world there's only so many that I can manage each week. It's also often a bit emotionally draining for me, since I don't enjoy saying no to PRs, or asking other volunteers to do even more work (e.g. making more changes to their PR). But rather than having fewer PRs opened each week (which would be a bad situation), I would like to get more reviews completed each week instead.

The best thing that you can do to help is to review someone else's PR. Don't worry, there's over 70 open today, I'm sure you can find one of interest to you! Please take a look at it, read through it, have a think about the problem and the proposed solution and see what comes to mind. Maybe you'll spot that there's actually an underlying problem that should be solved instead (https://github.com/openstreetmap/openstreetmap-website/pull/3731 is a great example of this), or you would have a different approach if you did it yourself, or you know some of the guidelines for making good PRs and can help other contributors shape up their PR. That way the PR will become easier for the maintainers to review and merge when they get to it.

All of this helps share the workload for the maintainers. And it's the main path to becoming a maintainer yourself! I would also love to see a few more maintainers in the team.

I'm happy to help anyone who is helping out with the work of reviewing PRs, since I think it's the main thing that is limiting our progress right now.

[lectrician1]

I personally do not have the motivation to review PRs because I don't know the site architecture, Ruby, and really am only concerned with features that I would like to be implemented. I'm sure that many others in the community are in the same position and I don't think we should expect them or even you or @tomhughes to continue to voluntarily help the website and other OpenStreetMap software to remain up-to-date with proposed changes.

To me, given the number of PRs that need to be reviewed and want from the community for large software changes for the site to be made yet nothing happens, it would make sense to have a paid maintainer/feature developer for the OpenStreetMap website and possibly other OSM software that needs to be kept up-to-date like the Wiki. Thoughts?

[tomhughes]

How is that supposed to work? That just makes the problem worse by ensuring that there will be even more PRs for us to review unless you are proposing that this developer should be able to push changes without review?

The big problem with the current review queue is that a lot of them are client side so I've mostly been leaving them to @gravitystorm to review - rails side stuff I can usually manage to review so long as it's not massive.

[lectrician1]

proposing that this developer should be able to push changes without review?

Hm true. That wouldn't be good. Well, to solve the software development deficiency, maybe the EWG should make more work on the site project grant-based like they are doing right now with the block feature. I don't think that would be reasonable for smaller site improvements though. If we'd like small improvements to be made too, an additional developer could be hired and the maintainer could then review their code.

[pnorman]

an additional developer could be hired and the maintainer could then review their code

The issue raised above is that the maintainers can't keep up with the code reviews required, and needing more non-maintainer reviews. Your suggestion would not help with that, in fact, it would increase the workload.

[lonvia]

You could still pay somebody to do simple PR triage as suggested by @gravitystorm. With PRs dating as far back as 2013 there is a lot of boring groundwork of finding out if it is still workable/relevant. In fact, it might be a good way to get somebody started that can then grow into the role of paid supporting developer.

[lectrician1]

The issue raised above is that the maintainers can't keep up with the code reviews required, and needing more non-maintainer reviews. Your suggestion would not help with that, in fact, it would increase the workload.

Well I'm saying you would have a paid maintainer in-addition to a paid developer.

[tomhughes]

Maintainership is something that needs to come from the establishment of trust over a period of time though - it's not something you can just appoint somebody to on a whim.

The existing maintainers will need to be happy that any proposed new maintainer has similar goals and expectations basically - you can't have code maintained by multiple people if they are making decisions at odds with each other.

[mmd-osm]

I don't believe reviewing random pull requests is an effective way going forward. I doubt it will add much value, even worse it might only add more noise without getting anything done in the end. Oftentimes, I find myself wondering, what maintainers think about a particular pull request, if it's even worthwhile reviewing, when in the end, no one wants that feature.

I'd like to propose a different approach where we can focus on a very limited set of pull requests or one topic to be completed in a ~3 week period. Then tell everyone, hey, this is what I'd like to work on, please help reviewing, add your comments. Work packages (or backlog items) have to fit into the 3 week time window, otherwise they're too large, and not a good fit. Use Github projects to communicate this planning, assign pull requests to different stages as needed.

I also agree you should urge EWG to find more resources to work on these topics. There's much more topics beyond reviewing PRs. The new microcosm could benefit from some UX experts, or find someone to sort out what features are useful or relevant for the site, as an example.

[gravitystorm]

I just want to say here thanks to @pnorman and @mmd-osm for their reviews over the last few weeks, I've found them very helpful. In particular it's helpful to know when a PR might have been overlooked and is otherwise straightforward to review or ready for merging.

I don't believe reviewing random pull requests is an effective way going forward. I doubt it will add much value, even worse it might only add more noise without getting anything done in the end.

Well, I disagree with you there! It does add value, since I think you have good judgement. But more importantly, you often highlight things that need dealing with or look at something from an angle that the OP might not have considered. So I don't think you would be just adding more noise.

Then tell everyone, hey, this is what I'd like to work on, please help reviewing, add your comments.

I think this sentence is directed at the maintainers, as opposed to the OPs, right? As in you think a maintainer could shortlist one or more PRs for closer inspection, we all then focus on just those PRs? It's an interesting idea, and not an approach that I've considered before, so I'll have a think about it. My first concern is that it treads close to asking volunteers to do work that they aren't interested in. For example, if a reviewer normally concentrates on javascript stuff, and I don't put any javascript-related PRs on the shortlist, could they then be discouraged from participating for those three weeks?

Hopefully we'll reduce the number of PRs in the backlog where such shortlisting and planning overhead wouldn't be necessary.

[gravitystorm]

I personally do not have the motivation to review PRs because I don't know the site architecture, Ruby, and really am only concerned with features that I would like to be implemented.

Ah, that's unfortunate. It would be great to have a think about that approach, and maybe try dipping your toes into parts of the project that you are less familiar with - or at the least, PRs by other people that are touching the parts of the codebase that you are already making PRs for.

If everyone took up the approach of only being concerned with the features that they are personally interested in, then that would be very limiting in what would get done - I'd certainly be reviewing far fewer PRs, for example! Part of teamwork is sometimes doing a little bit of something you aren't particularly interested in, in order to help each other out with their own priorities. After all, someone else has to help you with implementing the features that you are interested in too, right?

[angoca]

What are the guidelines for writing new features for the website?

As a person that has solved thousands of notes, I wrote an issue, and I was creating a pull request for a slight change but significant improvement in this area; however, I have received a message from a maintainer saying Stop with that obsession. What is this kind of communication for community building around website improvement? Does this motivate you to continue coding for the website?

The feature received positive feedback from note solvers, but we were ignored. Also, I created a thread in communities with the same support from the community.

[tomhughes]

It's not really possible to document what is and is not acceptable.

I mean obviously you can document some things which are definitely out of scope but every change has to be considered on it's merits at the time it's made and we're not going to write some list of rules and say if you comply with all these we guarantee to take your pull request.

I did try and explaining my reasoning on the issue you mention, that it was a deliberate design decision for notes to not carry a lot of meta data (and disguising it as a hash tag in the body doesn't stop it being meta data, it just makes it poor meta data) but maybe I wasn't clear enough. Obviously we can revisit that decision as it was taken quite a long time ago but that is a pre-existing design rationale.

[woodpeck]

What are the guidelines for writing new features for the website?

A good first step is discussing a problem and possible solutions before embarking on a solution. "Community building around website improvement" doesn't happen by people throwing a pull request over the fence that scratches their itch. (In this particular situation, there was a discussion and many people said that simply adding a hashtag was a bad idea, and you ignored that in your pull request.)

The message you have received was not to "stop with that obsession" but to stop with the hashtag obsession. Hashtags have a difficult history in OSM. A large number of changesets nowadays contain only hashtags as comments because people seem to believe that the main purpose of changeset comments is automatic parsing for leaderboards in task managers. That's why many people are skeptical when hashtags are presented as a quick cure for some problem.

While +1's from community members who do not participate in code maintenance show some support for your idea, these community members will usually not be able to assess the architectural/technical consequences of an idea (and will not be the ones who have to live with the code going forward).

[gravitystorm]

Please help us keep this issue on-topic. The topic for this issue is how to help us with processing our backlog of pull requests. It's not an appropriate place to discuss how to implement new features.

[Firefishy]

Tiny contribution. Attached is a list of the current tickets and PRs as of today. Others may also find this useful to parse via a spreadsheet:

openstreetmap-website_issues.csv