Benchmark results

Instruction counts

Significant differences

⚠️ There are significant instruction count differences

Other differences

Wall-time

Significant differences

There are no significant wall-time differences

Other differences

Memory usage

Key:

• ∑: sum usage for entire benchmark run
• 🔝: peak usage
• B: bytes
• a: allocations

Significant differences

There are no significant memory usage differences

Additional information

Historical results

Checkout details:

• Base repo: https://github.com/rustls/rustls.git
• Base branch: rel-0.23 (aff9845)
• Candidate repo: https://github.com/rustls/rustls.git
• Candidate branch: prepare-0.23.42 (e1c2325)

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.32%. Comparing base (aff9845) to head (e1c2325).

@@            Coverage Diff            @@
##           rel-0.23    #3143   +/-   ##
=========================================
  Coverage     95.32%   95.32%           
=========================================
  Files            97       97           
  Lines         21979    21979           
=========================================
  Hits          20951    20951           
  Misses         1028     1028           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

I think this should have proposed release notes as part of the PR since this was adding user-facing API surface per the RELEASING.md documentation.

I tried to backfill some into the release. Further edits welcome.

IMO this is pretty niche, even if it's user facing, and it's hard to miss in the short release notes.

I don't think it's all that more niche than things we've mentioned in other release notes ("Alter default of ServerConfig::require_ems based on provider's FIPS status.", "allow skipping selected ALPN validation", etc).

Yes, I would probably not write up separate release notes for those either. (Especially if there aren't many other PRs in the release.)

I guess I see the point of proposing release notes (or proposing there's none worth adding) is that it allows us to have this discussion. For example the FIPS one came from #3044 and was approved as-is.

I guess I see the point of proposing release notes (or proposing there's none worth adding) is that it allows us to have this discussion. For example the FIPS one came from #3044 and was approved as-is.

I didn't write anything in this PR and it was still approved, does that count as an implicit discussion?

I want to keep these bugfix releases cheap, and if there's a small list of PRs it's pretty easy to chase down what changed already. IMO writing up custom notes is mainly worth it for larger releases to call out user-visible behavior changes.

I didn't write anything in this PR and it was still approved, does that count as an implicit discussion?

Ideally ctz or myself would have asked about release notes before approving.

It's a minor thing but I think if we have a release process written down it's to make releases consistent and we should try to follow the documented procedure, or fix what's documented when it doesn't match how we want to do releases in practice.

I want to keep these bugfix releases cheap, and if there's a small list of PRs it's pretty easy to chase down what changed already. IMO writing up custom notes is mainly worth it for larger releases to call out user-visible behavior changes.

That's fair, but it also took me very little time to write the custom release notes here (and I would have appreciated them as a consumer of the release for rustls-ffi). I don't think asking for proposed release notes would make the process particularly expensive (especially for releases like this one where there were few changes to dig through), but if it's a blocker I'm also happy to try and suggest release notes so you don't have to.

I didn't write anything in this PR and it was still approved, does that count as an implicit discussion?

Ideally ctz or myself would have asked about release notes before approving.

It's a minor thing but I think if we have a release process written down it's to make releases consistent and we should try to follow the documented procedure, or fix what's documented when it doesn't match how we want to do releases in practice.

I think this is the relevant section from RELEASING.md:

Come up with text detailing headline changes for this release. General guidelines:

💚 include any breaking changes.
💚 include any major new headline features.
💚 include any major, user-visible bug fixes.
💚 include any new API deprecations.
💚 emphasise contributions from outside the maintainer team.
❌ omit any internal build, process or test improvements.
❌ omit any minor or user-invisible bug fixes.
❌ omit any changes to dependency versions (unless these cause breaking changes).

IMO this change is not breaking and is not major, so the decision not to include it seems in line with these guidelines?

I want to keep these bugfix releases cheap, and if there's a small list of PRs it's pretty easy to chase down what changed already. IMO writing up custom notes is mainly worth it for larger releases to call out user-visible behavior changes.

That's fair, but it also took me very little time to write the custom release notes here (and I would have appreciated them as a consumer of the release for rustls-ffi). I don't think asking for proposed release notes would make the process particularly expensive (especially for releases like this one where there were few changes to dig through), but if it's a blocker I'm also happy to try and suggest release notes so you don't have to.

I think the added value of the custom release notes for a case like this is very small, since a small percentages of users are likely to look at the changelog for semver-compatible releases in the first place and the changes included are still legible via the list of PRs (in this case, linking back to earlier PRs and the issue). I'm fine if you want to asynchronously add to the release notes, but I would prefer not to block on it or add round trips for it, since I want to prioritize shipping changes rather than meta-documenting them.

I think a new TLS protocol feature should count as a major change (as, theoretically, it can cause interop problems that causes someone to look into their dependency changes closely.) Sorry for missing that!

as, theoretically, it can cause interop problems that causes someone to look into their dependency changes closely.)

Though, somewhat unlikely as it is not activated by default for clients.

unlikely as it is not activated by default for clients.

Right, and that's the sort of detail worth capturing in the notes IMO.

I think ctz and I are on the same page that implementing a new RFC with client/server API surface is probably worth release note mention and we can try and provide that perspective for discussion on future prep PRs when applicable.

I don't want to rules lawyer our own release notes guidance and if it's OK to asynchronously add release notes where I think they might be worthwhile I'm happy to call this sorted.

Thanks for the discussion