-
Notifications
You must be signed in to change notification settings - Fork 238
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
Merge in backend-explicit-tests #129
Conversation
NOTE: This identified a few bugs with the python-rsa and pyca/cryptography backends. Those will fail for now.
Cryptographic backend isolation tests
test_pycrypto_RSA_key_instance and test_cryptography_RSA_key_instance both test compatibility of the backend-explicit RSAKey classes with the backend-native RSA key structures. As such, these tests must use the backend-explicit RSAKey classes rather than the default loaded from jose.backends to avoid breaking when multiple backends are present.
commands_pre requires tox >= 3.4.0
Python-rsa does not support certificates.
…mpatibility between backends
… for known legacy prefix in legacy parsing
[RSA] make backend-explicit tests use backend-explicit keys
disable Firebase tests on python-rsa backend
Remove pyca/cryptography backend's dependency on python-ecdsa
Fix invalid RSA private key PKCS8 encoding
…g on the backend used
make cryptography_backend the default for RSAKey
…y on Crypto.IO module
…lling dependencies directly
Remove pycrypto/dome dependency on python-rsa
Prep backend-explicit-tests for merge
Codecov Report
@@ Coverage Diff @@
## master #129 +/- ##
==========================================
+ Coverage 94.6% 96.52% +1.92%
==========================================
Files 13 14 +1
Lines 1000 1065 +65
==========================================
+ Hits 946 1028 +82
+ Misses 54 37 -17
Continue to review full report at Codecov.
|
I would like to make a release that includes these changes (as well as your incoming readme PR), and my Ed25519 support PR once that is merged (I’ll have time to finish that up later this week). So in terms of time, I would estimate a release within 1-2 weeks, if @zejn doesn’t have any objections. I’ll also triage some of the outstanding PRs and issues a bit more later this week so they make it into the next release. In terms of a versioning method, I haven’t taken a look at previous release version strings, but I imagine that SemVer is probably a good default versioning scheme. Finally, since @zejn is a more familiar with your previous PR I’ll leave this for him to merge. Thank you for your contributions so far and keep up the good work both of you! Ninja edit: changelog -> readme |
I don't think there's currently an explicit versioning policy. Whenever there were bigger changes, there was a major version increase. I don't think current changes need a major version increase, I'd make it a The priority of the backends was changed, yes, but that should not make an existing installed backend unimportable and nonfunctional. If that happens, then it's a bug, but not an API breakage. I'd also like to see issue #112 fixed before the next release and PR #100 has seen a lot of work and is nearing readiness. I would like to hear what @mpdavis or @blag think about this though. |
This looks good and seems we agree with @blag on a rough release schedule. |
Ok, now the big one.
I am intentionally merging from the
mpdavis
fork to verify that nothing was changed on my end. All of these changes have been handled through PRs into thempdavis/backend-explicit-tests
dev branch, terminating in a final merge ofmaster
intobackend-explicit-tests
.I also have a readme change that I want to make to better frame these changes once they are released, but I'll get that into
master
after this.Open question to the maintainers @zejn @mpdavis : what version will the first release to incorporate these changes be? I don't see a versioning policy explicitly stated anywhere, and as noted in #99 there aren't currently any release notes that I could look though to infer a versioning policy. Would the next release be a good time to address both of those gaps?