Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn about missing algorithms arg only when verify is True #281

Conversation

suligap
Copy link
Contributor

@suligap suligap commented Jul 4, 2017

Since no signature verification will occur, passing in algorithms does
not make much sense.

@coveralls
Copy link

coveralls commented Jul 4, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 9e63858 on suligap:no-alg-deprecate-warn-only-when-verify-is-true into 74399b1 on jpadilla:master.

@mark-adams
Copy link
Contributor

Might not hurt to add a test for this.

@suligap suligap force-pushed the no-alg-deprecate-warn-only-when-verify-is-true branch from 9e63858 to 4b16fe8 Compare July 6, 2017 09:22
@coveralls
Copy link

coveralls commented Jul 6, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 4b16fe8 on suligap:no-alg-deprecate-warn-only-when-verify-is-true into 74399b1 on jpadilla:master.

@suligap
Copy link
Contributor Author

suligap commented Jul 6, 2017

@mark-adams thanks. Tests added (and the original change fixed).

@mark-adams
Copy link
Contributor

@suligap Sorry for the delay in reviewing. This looks good. Would you mind adding an entry to the CHANGELOG mentioning the behavior that this fixes?

Copy link
Contributor

@mark-adams mark-adams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if you add this to the CHANGELOG we should be good!

@suligap suligap force-pushed the no-alg-deprecate-warn-only-when-verify-is-true branch 2 times, most recently from cabc3ee to 0fa05a6 Compare August 28, 2017 11:28
@coveralls
Copy link

coveralls commented Aug 28, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 0fa05a6 on suligap:no-alg-deprecate-warn-only-when-verify-is-true into 3def8d8 on jpadilla:master.

@suligap
Copy link
Contributor Author

suligap commented Aug 28, 2017

@mark-adams thanks. CHANGELOG updated!

- Remove uses of deprecated functions from the cryptography package.
- Warn about missing `algorithms` param to `decode()` only when `verify` param is `True` [#281][281]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing defining the link for 281 at the bottom of the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jpadilla! That's fixed now.

Since no signature verification will occur, passing in `algorithms` does
not make much sense.
@suligap suligap force-pushed the no-alg-deprecate-warn-only-when-verify-is-true branch from 0fa05a6 to 1b57ab2 Compare August 29, 2017 07:41
@coveralls
Copy link

coveralls commented Aug 29, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 1b57ab2 on suligap:no-alg-deprecate-warn-only-when-verify-is-true into 3def8d8 on jpadilla:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants