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

Add settings to CredentialPayload #23690

Merged
merged 6 commits into from
Sep 16, 2017
Merged

Add settings to CredentialPayload #23690

merged 6 commits into from
Sep 16, 2017

Conversation

omus
Copy link
Member

@omus omus commented Sep 13, 2017

Allows users to control whether the SSH agent or prompting should be allowed when authenticating. Removes the last piece of payload state stored in UserPasswordCredentials and SSHCredentials. With the removal of prompt_if_incorrect we can deprecate most of the original constructors.

Part of #20725.

@omus omus added deprecation This change introduces or involves a deprecation libgit2 The libgit2 library or the LibGit2 stdlib module labels Sep 13, 2017
@omus omus force-pushed the cv/libgit2-allow-keywords branch 2 times, most recently from fea2da7 to 0e307e3 Compare September 14, 2017 18:54
@omus
Copy link
Member Author

omus commented Sep 14, 2017

Added some comments for the allow fields. I also did a rebase and dealt with the NEWS conflict.

Copy link
Contributor

@rofinn rofinn left a comment

Choose a reason for hiding this comment

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

I'm not sure how much people have been depending on the internal LibGit2 implementation, but I'd clarify a few more points in the NEWS.md just in case. Apart from that I think this is good to merge after a rebase with passing tests.

NEWS.md Outdated
@@ -443,6 +443,10 @@ Deprecated or removed
this is not true. If you are certain you need the old behavior, it is temporarily available
as `Base._isleaftype` ([#17086]).

* Constructors for `LibGit2.UserPasswordCredentials` and `LibGit2.SSHCredentials` no longer take a
`prompt_if_incorrect` argument. Controlling prompting behavior has been moved to
`LibGit2.CredentialPayload`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to also clarify a few points:

  1. prompt_if_incorrect has also been renamed to allow_prompt in the CredentialPayload
  2. use_ssh_agent is now a Bool vs Char.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll update the news to mention allow_prompt. As for change to use_ssh_agent in CredentialPayload I'll avoid mentioning that since the entire CredentialPayload is new to 0.7.

Removes `prompt_if_incorrect` field from `SSHCredentials` and
`UserPasswordCredentials` and replaces it with the `allow_prompt` field
in `CredentialPayload`. Note that allowing prompting is now the default.

Note: Although we can deprecate when people specify a boolean for
the `prompt_if_incorrect` parameter we cannot detect if people were
reliant on prompting being disabled by default.
@omus omus force-pushed the cv/libgit2-allow-keywords branch from 0e307e3 to e27c77d Compare September 15, 2017 20:19
@omus omus merged commit 9e3ec21 into master Sep 16, 2017
@omus omus deleted the cv/libgit2-allow-keywords branch September 16, 2017 23:32
omus added a commit that referenced this pull request Sep 18, 2017
Requires:
- Approval/rejection (#23711)
- Allow prompt keyword (#23690)

Helpful:
- Generate tests (#23668)

I would apply this commit to the approval/rejection PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation This change introduces or involves a deprecation libgit2 The libgit2 library or the LibGit2 stdlib module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants