Fix username retrieval #45
Conversation
src/lib.rs
Outdated
| @@ -132,14 +121,15 @@ impl CredentialHandler { | |||
| // handle this we bail out of this authentication session after setting | |||
| // the flag `ssh_username_requested`, and then we handle this below. | |||
| if allowed.contains(git2::CredentialType::USERNAME) { | |||
| // debug_assert!(username.is_none()); | |||
| debug_assert!(username.is_none()); | |||
There was a problem hiding this comment.
I don't want the lib to panic if username is defined. Can you comment the statement again or handle this case differently?
There was a problem hiding this comment.
Thank you for reviewing!
fixed by a2ee307
src/ssh_config.rs
Outdated
| if key.as_str().eq_ignore_ascii_case(name) { | ||
| let path = value.as_str().to_string(); | ||
|
|
||
| // trace!("found IdentityFile option with value {:?}", path); |
There was a problem hiding this comment.
As you modify the function (and this part) to not only retrieve a path of IdentityFile, can you also rename the variable path to reflect this change eg found_value).
Maybe in the future, an optimisation will be to not parse the file for every search.
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more. @@ Coverage Diff @@
## master #45 +/- ##
==========================================
+ Coverage 87.38% 89.34% +1.95%
==========================================
Files 2 2
Lines 111 122 +11
==========================================
+ Hits 97 109 +12
+ Misses 14 13 -1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Address the review comment: davidB#45 (comment)
Address the review comment: davidB#45 (comment)
|
I release your fix as part of version 0.12.1 (for git2 0.17) and 0.13.0 (for git2 0.18). Thank you. |
Fixes #42, #44