OpenSSL Communities

OpenSSL PR review process

DB Dmitry Belyavsky Mon 5 Jan 2026 10:17AM Public Seen by 30

Currently the PR review process is completely broken.

We have ~350 PRs, the oldest ones dating back to 2018 (and I don’t believe they are revivable).

It’s very problematic to predict whether the PR will be initially evaluated in a reasonable time. And even after that it’s not always possible to get a review.

I’m speaking mostly for external contributions, I believe that for the internal PRs (raised by OpenSSL Corporation/Foundation people) there is some process to draw attention of other people.

Currently it’s impossible to say whether PR got any attention at all. We need to establish a process that would allow PRs to move forward in a predictable manner. 

Goals

Simple PRs should move forward fast.

Complex PRs should move forward.

The contributors should know that the PR is blocked on them.

PR "substatuses" and proposed dealing with it

This section lists various substatuses without any particular order and provides (manual) actions to be taken either by committers or by the contributor. The list is incomplete.

Draft PRs are out of scope of this section.

A new PR should be labeled for the branches it affects, and triaged (one of “triaged” labels should be applied). Also triaging should require setting hold labels in case of controversial changes, lack of tests or documentation.

A PR from a completely new contributor is automatically labeled “cla: required” and requires manual approval for running tests. I think we should stop requiring manual approval after the ICLA is signed, if it’s possible using automation. 

A PR failing the tests in theory should be returned back to the contributor to deal with failing tests. Unfortunately, we have some random failures, we may have incorrect labeling for the versions requiring backports etc.

A PR that has conflicts needs rebase, and contributors should be notified about it.

A PR that has unresolved change requests should be dealt with by contributors.

PRs that pass all tests except known flaky ones should get a reviewer assigned. 

Proposed process for new PRs (younger than 180 days)

A PR that requires action from committers, should be triaged by a person on duty (all committers? Foundation resources?). 

In case when tests are passed, not later than in a week at least one reviewer should be assigned (the procedure TBD). After the 1st approval, not later than in a week the 2nd reviewer should be assigned (the procedure TBD). 

Personally I believe that the committers should have a real-time communication channel for assigning reviewers and ad-hoc discussions.

Proposed process for stale PRs

Close them all. Let’s review them one-by-one, bury the abandoned ones and irrelevant ones, and deal with the remaining ones. In case when the original contributor doesn’t respond but has signed a CLA, I believe we can reuse the contributions and adjust them. The re-triaging procedure TBD

Automation adjustments

Currently a significant share of email notifications regarding stale PRs is misleading - the PRs I get notifications about as “an actions of committers required” quite often requires actions from the contributor. Also most of these emails are just ignored. So we probably should use some dashboards instead.

Contributors should be notified in case when their PRs need rebasing/separate backporting/etc. We probably should have separate procedures for libcrypto.num/CHANGES.md conflicts and deal with them on merge.

There is also a script https://github.com/baentsch/openssl/blob/mb-gitanalysis/gitanalysis/checkissues.py  written by Michael Baentsch that provides some relevant insights regarding the PRs status; it can be adjusted especially for dealing with stale PRs.

NH

Neil Horman Mon 5 Jan 2026 2:19PM

I'm generally in favor of increased rigor around our PR/issue management process. Some notes regarding the above proposal however:

A new PR should be labeled for the branches it affects, and triaged (one of “triaged” labels should be applied). Also triaging should require setting hold labels in case of controversial changes, lack of tests or documentation.

This exists as current process now, as part of the bug wrangler duties

I think(?) the issue with execution on this task is more an issue of us not having an assigned person or persons to the task on a regular basis

A PR from a completely new contributor is automatically labeled “cla: required” and requires manual approval for running tests. I think we should stop requiring manual approval after the ICLA is signed, if it’s possible using automation.

Currently, github offers 3 repository setting states that affect this:

1) Users who have recently been created on github and have never had a PR merged to our repo require manual CI starts

2) Users who have never had a PR merged to our repo require manual CI starts

3) Any user opening a PR will automatically start CI

We're currently set to option (2), which seems like a reasonable setting, barring other concerns. If we resolve to have a person/people consistently assigned to do bug wrangling above, I think this can be addressed as part of those duties.

A PR failing the tests in theory should be returned back to the contributor to deal with failing tests. Unfortunately, we have some random failures, we may have incorrect labeling for the versions requiring backports etc.

A PR that has conflicts needs rebase, and contributors should be notified about it.

A PR that has unresolved change requests should be dealt with by contributors.

All of these seem reasonable, and I would go so far as to say, already implicitly assumed by the body of committers (speaking for myself, I work under the assumption that failing CI, conflicts and change requests are generally blockers to merge). I think the big problem here is that github kind of stinks in this area. It has no concept of "Waiting on" - i.e. theres no clear way for all participants to know what the expectation is for "what hapeens next". We could certainly try to add something (the most direct thing I think, being a board that all pr's get added to with a waiting on state variable), but I'm not sure whats best here.

A PR that requires action from committers, should be triaged by a person on duty (all committers? Foundation resources?).

In case when tests are passed, not later than in a week at least one reviewer should be assigned (the procedure TBD). After the 1st approval, not later than in a week the 2nd reviewer should be assigned (the procedure TBD).

Personally I believe that the committers should have a real-time communication channel for assigning reviewers and ad-hoc discussions.

The triaging I think is solved by an assigned wrangler (see above). As for reviewer assignments, I think we had a similar discussion here a while back, talking about how to handle reviews when we don't have clear subject matter expertise in a given code area. that is to say a code-owners/MAINTAINERS file, that is filled out such that comitters/contributors are robustly tracked in such a file, helps automate these assignments. That said, I'm not sure the problem we need to tackle here is one of assignment, but rather one of accountability. That is to say that doing reviews is something that, for better or worse, a best effort work item. If I don't get to a review, thats unfortunate, but ultimately, not impactful to me personally. the reviews I get to consistently are the ones that I'm asked about on our daily standup (something as simple as being reminded a few days in a row, keeps it at the front of my mental to do list enough that I get them done). Perhaps what we need here is some sort of simmilar motivator, something like a scoreboard that shows PR's listed oldest to newest, with reviews yet to be completed, so we know when we as individuals are holding up the train.

Let’s review them one-by-one, bury the abandoned ones and irrelevant ones, and deal with the remaining ones. In case when the original contributor doesn’t respond but has signed a CLA, I believe we can reuse the contributions and adjust them. The re-triaging procedure TBD

We've actually already done this (or half of it). For issues, we labeled everything we considered stale as inactive, and closed them at the end of 3.4 development. That netted about 810 closed issues, which is great, but we still have around 1300 open issues. We could certainly do the same thing we PRs (I would be supportive), but it seems like the sort of thing that just grows back if we don't continually address it). As a note, if we could solve the "waiting on" issue above, I would be supportive of an automation that auto-closes any issue or PR that sits on "waiting on author/contributor" for more than 180 days (or insert your stale borderline here) to address this in an ongoing manner.

DB

Dmitry Belyavsky Mon 5 Jan 2026 4:00PM

@Neil Horman
As for reviewer assignments, I think we had a similar discussion here a while back, talking about how to handle reviews when we don't have clear subject matter expertise in a given code area.

The problem is that even the PRs we have expertise still stay in limbo - I regularly pick a low-hanging fruit from a daily pile "This PR requires committer's action" emails and they often are not rocket science at all

Thanks for pointing out to the other weak points/platform limitations. The proposal is intended to be a starting point for the discussion, not a final decision.