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

Remove outdated LibGit2.get_creds! methods #23286

Merged
merged 3 commits into from
Aug 22, 2017
Merged

Remove outdated LibGit2.get_creds! methods #23286

merged 3 commits into from
Aug 22, 2017

Conversation

omus
Copy link
Member

@omus omus commented Aug 17, 2017

Part of #20725.

Did a cleanup pass after #23189.

  • Moved the AbstractCredentials closer to the other credentials. Previously this code needed to be defined before a RemoteCallbacks constructor that no longer exists.
  • Deprecated Remove some get_creds! methods which are no longer required since the introduction of the CredentialPayload.

@omus omus added the libgit2 The libgit2 library or the LibGit2 stdlib module label Aug 17, 2017
@omus
Copy link
Member Author

omus commented Aug 17, 2017

I'm not sure what the protocol is when we're removing a method with no replacement. I'm assuming it should still go through a deprecation cycle.

@Keno
Copy link
Member

Keno commented Aug 17, 2017

If it wasn't exported, it doesn't need a deprecation cycle.

@omus
Copy link
Member Author

omus commented Aug 17, 2017

Unfortunately most things in the LibGit2 module aren't exported so it can be difficult to know what is being used externally. I'm pretty sure these methods are not be used externally so I'll remove the "base/deprecated.jl" changes.

@omus omus changed the title Deprecate some LibGit2.get_creds! methods Remove outdated LibGit2.get_creds! methods Aug 17, 2017
@omus
Copy link
Member Author

omus commented Aug 17, 2017

Removed the deprecations.

@tkelman
Copy link
Contributor

tkelman commented Aug 17, 2017

What's the "too many prompts" error on circle about?

Agreed that if things are externally used then it's nice as a courtesy to deprecate instead of hard break, when possible. If not externally used then it's not too important.

@omus
Copy link
Member Author

omus commented Aug 17, 2017

What's the "too many prompts" error on circle about?

That is coming from the challenge_prompt and typically only appears during a LibGit2 test failure. It is possible however that this is a false positive as the way we detect "too many prompts" by using a timeout of 10 seconds.

Is there a reason there isn't a full traceback on Circle CI?

@omus
Copy link
Member Author

omus commented Aug 17, 2017

I've triggered a rebuild to see if it reoccurs. The failing Circle CI log can be seen here.

@tkelman
Copy link
Contributor

tkelman commented Aug 17, 2017

dunno, would we get a better backtrace on that kind of failure on travis? it might be something strange with piping or where stderr goes, or it may relate to #18647?

@omus
Copy link
Member Author

omus commented Aug 18, 2017

On Travis CI I typically see full stack traces for those kind of exceptions. I'm not going to worry too much about it as it's only occurred once.

Any other comments?

@omus
Copy link
Member Author

omus commented Aug 20, 2017

Removed one more unused method. I also changed the first commit message as it referred to deprecating a method instead of removing it.

@omus omus merged commit cf1ae29 into master Aug 22, 2017
@omus omus deleted the cv/cred-cleanup branch August 22, 2017 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libgit2 The libgit2 library or the LibGit2 stdlib module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants