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(utils): add support for PAT based remote URLs #90

Merged
merged 4 commits into from
Feb 15, 2022
Merged

fix(utils): add support for PAT based remote URLs #90

merged 4 commits into from
Feb 15, 2022

Conversation

burntcarrot
Copy link
Contributor

@burntcarrot burntcarrot commented Feb 14, 2022

The current method of extracting owners from remote URLs doesn't work with remote URLs having a PAT in it, for example:

https://username:<access_token>@github.com/username/repo

This can be improved by using a regular expression like this, which can extract owner names from both type of remote URLs, as shown here.

Signed-off-by: burntcarrot aadhav.n1@gmail.com

@cla-bot
Copy link

cla-bot bot commented Feb 14, 2022

Thanks for your pull request. Before we proceed, you"ll need to sign a Contributor License Agreement - https://docs.google.com/document/d/1Wj255zTFkO0XpC7piChAa0yd96KCEqXf4Qa3e87F5UA. Please send the signed copy to jai@deepsource.io to proceed with the patch review.

* add regular expressions to extract owner from remote URL
* fix typos

Signed-off-by: burntcarrot <aadhav.n1@gmail.com>
@cla-bot cla-bot bot added the cla-signed label Feb 14, 2022
Copy link
Contributor

@siddhant-deepsource siddhant-deepsource left a comment

Choose a reason for hiding this comment

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

@burntcarrot Good catch and thanks for the patch. Would be great if you can add tests. 🚀

command/repo/status/status.go Show resolved Hide resolved
utils/fetch_remote.go Show resolved Hide resolved
* add mocks for commands
* add test for fetching remotes
* refactor remote fetching utility to adjust mocks
* refactor remote resolver

Signed-off-by: burntcarrot <aadhav.n1@gmail.com>
* refactor ListRemotes
* refactor tests
* add new test cases for SSH and multiple remotes
* move to field-based splitting instead of tab-based splitting

Signed-off-by: burntcarrot <aadhav.n1@gmail.com>
Signed-off-by: burntcarrot <aadhav.n1@gmail.com>
Copy link
Contributor

@siddhant-deepsource siddhant-deepsource left a comment

Choose a reason for hiding this comment

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

Great work @burntcarrot!! Let's go 🚀

@siddhant-deepsource siddhant-deepsource merged commit 8df07dd into DeepSourceCorp:master Feb 15, 2022
@burntcarrot burntcarrot deleted the fix-remote branch February 15, 2022 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants