-
Notifications
You must be signed in to change notification settings - Fork 307
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
Fixes #418 Support keyring.get_credential #419
Conversation
The docs failure is not my fault (perhaps I should write some docs...?) - it's in the contributors guide. |
Codecov Report
@@ Coverage Diff @@
## master #419 +/- ##
==========================================
+ Coverage 78.29% 78.29% +<.01%
==========================================
Files 14 14
Lines 737 751 +14
Branches 106 108 +2
==========================================
+ Hits 577 588 +11
- Misses 127 130 +3
Partials 33 33
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. On minor request.
tests/test_utils.py
Outdated
@pytest.fixture | ||
def keyring_missing_get_credentials(monkeypatch): | ||
""" | ||
Simulate that 'import keyring' raises an ImportError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This docstring needs to be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So does the name that gets deleted immediately below... guess that's why the coverage didn't hit 100%?
@jaraco why are you merging changes for this project? I don't believe you were ever explicitly made a maintainer or asked to help |
Fixes #418 Support keyring.get_credential
This requires keyring 15.2 or later.