Finding reviewers for PRs

What is the current process for finding reviewers/making sure PRs get reviewed?

Feels like the process in scala3/MAINTENANCE.md at main · scala/scala3 · GitHub is no longer followed as the linked spreadsheet: Issue Supervision Statistics - Google Sheets stops in 2024.

For example I created this PR Fix false exhaustivity warning for pattern returning NamedTuple by eejbyfeldt · Pull Request #23362 · scala/scala3 · GitHub 7 months back. The actual change is really small, so I think the review burden of going Yes/No for someone familiar with the code is really low. But it still requires the PR to get surfaced to the right person, which is what I suspect does not happen.

Feels like we should improve this process so we do not miss out on outside contributions.

1 Like

Hey Emil thanks for the question - while i cant answer for the current core team, I would propose that because you modified the Space engine, then suddenlty there is a percieved “expert required here” fear that perhaps whoever is left on core does not suppose themself qualified for

I have a stack of PRs waiting for review, some quite old.

My only New Year’s resolution was to help reduce the backlog, including but not limited to my own, by helping to review and also triage and answer questions, on the theory that reducing the overall workload will free up resources for the difficult work of ingesting contributions without compromising quality.

My other goal, which I haven’t fully put into practice, is to make a PR simple to review. That means no extraneous edits (a challenge for my itchy fingers), and also making a clear argument for why the PR is worth reviewing and answering any open questions about it. The reviewer should not have to be expert, but should be persuaded by some combination of ticket comments and code inspection that the PR pulls its weight.

Even one LOC or LOCC (lines of code changed, a coinage?) is a cost. The reviewer’s (or reviewers’) time is a fixed cost. Is it worth the risk of breakage and costing more time in future?

I’m pretty sure all my one–LOCC PRs broke something; that goes up exponentially for half-LOCC.

The real limitation on an “academic” project is that everyone is involved in academia and is probably stretched somewhat thin, across responsibilities for studying, teaching, and research. (That is, the limitation is not that the project has goals that are “merely academic”.)

It’s unreasonable to expect one person to take on management of the backlog, though it would be nice if one person were prominently assigned the duty of fielding requests for review. That would be like triaging issues, except triaging PRs means judging if it’s a simple review and who might be in the best position to take a look at it.

The other factor is that the team has internal discussions about priorities and future direction, and none of that is visible to the outside. (I think that is OK.) So the point person doing triage (of requests for review) must be behind that firewall and know who has exams, who is swamped, and maybe who knows what. They must also know who got a job and is not available to review any more.

If I felt I had a PR that “moved the needle”, I would want to at-notify that point person. I don’t remember off-hand if any of my open PRs even make the needle oscillate a little.

Obviously, the most important issue right now is that publishLocal is broken (because that affects me directly). I have no way of knowing if the team has a plan for that, or it’s blocked by some other work on the build, the library, or the doc tool, etc. The other limitation of an academic project is that the smart people on the team would say, just use cs to fetch your dependencies and run the scala task from sbt, what’s the problem? That anecdote is to illustrate that priorities are relative to immediate needs.

I plan to renew my resolution for Lunar New Year.

I think the PR just got lost. We usually assign someone to review, but looks like no one was assigned in your case. Feel free to ping us in those cases. Sorry for not looking into that sooner

The real limitation on an “academic” project is that everyone is involved in academia and is probably stretched somewhat thin, across responsibilities for studying, teaching, and research. (That is, the limitation is not that the project has goals that are “merely academic”.)

But there is at least some industry usage of Scala. It would be surprising to me if that could not be leverage to get involvement from people outside academia.

If I felt I had a PR that “moved the needle”, I would want to at-notify that point person. I don’t remember off-hand if any of my open PRs even make the needle oscillate a little.

To me there is some “death by a thousand cuts” going on when using Scala. So I don’t not clear to that a PR needs to move the needle on is own. But lots of small fixes together does.

Feel free to ping us in those cases.

I guess clear what us is referring to here. Anyone one the compiler team?

You can always check who to ping on Scala Core Team | The Scala Programming Language

Either the product owner or coordinator is probably the person to ping. (Me or Piotr currently)

2 Likes