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 Git credential cache as backing store on Linux #287

Merged
merged 17 commits into from
Feb 22, 2021

Conversation

vtbassmatt
Copy link
Contributor

@vtbassmatt vtbassmatt commented Feb 12, 2021

Fixes #261

  • Factor out helper invoker
  • Invoke Git helpers using the correct instance of Git
  • Add config for using Git credential cache
  • Manual testing
    • Simple case: does it store and reuse a credential?
    • After running git credential-cache exit, are credentials dropped?
    • Passing arguments like --timeout via environment variable works?
  • Add unit tests
  • Include in documentation

@vtbassmatt vtbassmatt added the enhancement New feature or request label Feb 12, 2021
Copy link
Collaborator

@mjcheetham mjcheetham left a comment

Choose a reason for hiding this comment

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

Looks good so far.. some comments.

@vtbassmatt vtbassmatt marked this pull request as ready for review February 18, 2021 16:44
@vtbassmatt
Copy link
Contributor Author

Tested on an Ubuntu 20.04 machine against a private Azure Repos instance. I don't have a clear idea of how or what needs unit testing, and suggestions would be most welcomed.

Copy link
Collaborator

@mjcheetham mjcheetham left a 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! Left a question on the value of the store option, and also we're missing updates to the other settings/reference docs for the new options.

In terms of unit tests.. I am not sure it's worth the effort here, given this is largely around process creation and stdin/out forwarding.

src/shared/Microsoft.Git.CredentialManager/Git.cs Outdated Show resolved Hide resolved
docs/linuxcredstores.md Outdated Show resolved Hide resolved
env var for config is still the long form, but the git option
is already namespaced to `credential` so it was also shortened.
Copy link
Collaborator

@mjcheetham mjcheetham left a comment

Choose a reason for hiding this comment

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

:shipit:

@mjcheetham mjcheetham merged commit e96e689 into master Feb 22, 2021
@vtbassmatt vtbassmatt deleted the git-credential-cache branch February 22, 2021 13:29
@mjcheetham mjcheetham mentioned this pull request Mar 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplify usage in Azure Cloud Shell
2 participants