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

Fix coursier credentials #700

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lgzy
Copy link

@lgzy lgzy commented May 16, 2022

Changes _repository_credentials to return a string surrounded with quotes as the result is passed to commandline as one of parameters in the form of ... --credentials <result string> and ends up being incorrectly parsed. The quotes enable it to be correctly parsed.

@lgzy lgzy requested a review from cheister as a code owner May 16, 2022 10:22
@google-cla
Copy link

google-cla bot commented May 16, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

For more information, open the CLA check for this pull request.

@lgzy lgzy force-pushed the fix-repository-credentials branch from fdd6da4 to 79a2a91 Compare May 16, 2022 10:25
Copy link
Collaborator

@shs96c shs96c left a comment

Choose a reason for hiding this comment

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

Nice catch. Thank you for the PR. The CI tests that are failing look related, but once they pass, I'll be happy to merge this :)

@shs96c
Copy link
Collaborator

shs96c commented Jul 5, 2022

Sorry for the slow response to your initial PR --- I missed this in my reviews, and only just saw it. I appreciate your patience!

@lgzy lgzy force-pushed the fix-repository-credentials branch from 5d850d6 to 7190473 Compare July 8, 2022 14:51
@lgzy lgzy force-pushed the fix-repository-credentials branch from 7190473 to fa1b0c8 Compare July 8, 2022 14:52
@lgzy
Copy link
Author

lgzy commented Jul 8, 2022

fixed the offending test. The current failures don't seem to be relevant?

@lgzy lgzy requested a review from shs96c July 8, 2022 14:57
@pcj
Copy link
Member

pcj commented Oct 24, 2022

@lgzy can you rebase this PR on latest master?

@cheister
Copy link
Collaborator

cheister commented Dec 3, 2022

I believe this was fixed with #721 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants