Allow plaintext to be "false" #17
Merged
Conversation
This file contains 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
|
This I will need to look at closer |
|
@timlegge Hey, don't suppose you've had any time to look through this PR? Thanks. |
|
@JRaspass it looks fine. I likely would have left t/51-gh7.t in place simply to show that the changes did not impact current test but I did confirm it so that's fine |
Rather than truth checking we check for definedness allowing zero and empty string to be encrypted, signed, etc.
JRaspass
force-pushed
the
false-messages
branch
from
6943a63 to
8796dda
Compare
|
Awesome, thanks. I've updated the comment in the test to point at this PR and marked it ready to review then. Did you want this commit to update the Changes? Or do you prefer to do them all at release time? |
|
i will add them at release time |
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.
At $work we ran into an issue where messages could end up being "false", especially if they come from an automated system, and therefore end up rejected by this library. This sprinkles a few
defined()checks to fix this and enhances our test coverage of this issue.I've marked this PR as draft as I feel it needs more discussion as it is a change of behaviour, at the very least it'll need an issue number to reference in the test.