Correct the docs on when constructing a Digest can fail - #109
Merged
Conversation
PR #92 made the `Digest` constructors partial, so construction raises when OpenSSL cannot allocate the context. The package docstring still said construction cannot fail and that the failure surfaces at the first `append` or `final`, and it left the constructors off the list of partial functions. Two unreleased release notes were wrong too. One said a digest that could not be created reports it at the first `append` or `final`; the other said construction now raises rather than returning a digest that fails on every later call. Before #92 a failed construction crashed, so both described a state that never shipped. Closes #106
Member
Author
|
Autonomous-mode decision log — PR-specific No release note. The wrong docs arrived with the still-unreleased #92, so nothing incorrect shipped to users; correcting them before release needs no note. Scope grew after the pre-PR review. I first fixed only the two statements #106 quoted. The reviewer found a third: a sibling note ("Make Digest and HmacSha256 raise on failure") said construction now raises "rather than returning a digest that fails on every later call." I checked the released 2.1.0 constructor — it was non-partial and dereferenced a possibly-null context, so a failed construction crashed. Both wrong clauses describe a state that never shipped, so I corrected both to say "rather than crashing," matching the code and each other. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR #92 made the
Digestconstructors partial, so construction raises when OpenSSL cannot allocate the context. Three statements were left describing the old behavior.ssl/crypto/crypto.ponysaid constructing aDigestcannot fail and that the failure surfaces at the firstappendorfinal, and it omitted the constructors from the list of partial functions..release-notes/next-release.mdhad two wrong clauses about the same thing. One said a digest that could not be created reports it at the firstappendorfinal; the other said construction now raises rather than returning a digest that fails on every later call. Before #92 a failed construction crashed, so both described a state that never shipped.Closes #106