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

Introduce safe host parsing function #63

Merged
merged 6 commits into from
Oct 8, 2024

Conversation

mouismail
Copy link
Contributor

@mouismail mouismail commented Jun 25, 2024

This pull request primarily focuses on improving error handling and code readability in the OAuth flow. The changes include the introduction of a new function NewGitHubHost in oauth.go to parse the host URL and return an error if parsing fails. This function is then used in the files examples_test.go, oauth_device.go, and oauth_webapp.go to replace the previous method of creating a new host, improving error handling in the process.

Fixes: #62
Related: cli/cli#9248

oauth.go Show resolved Hide resolved
oauth.go Outdated Show resolved Hide resolved
examples_test.go Outdated Show resolved Hide resolved
oauth.go Show resolved Hide resolved
Copy link
Contributor

@BagToad BagToad left a comment

Choose a reason for hiding this comment

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

👋 Hey @mouismail. Sorry for the delays getting to this. We reviewed this and added some comments and suggestions. Let us know if we can help with anything!

Copy link
Contributor

@BagToad BagToad left a comment

Choose a reason for hiding this comment

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

Thank you again for your contribution @mouismail

I'll get this merged...

@BagToad
Copy link
Contributor

BagToad commented Oct 8, 2024

Ignoring the two CI tests for MacOS which are likely caused by old actions versions...

@BagToad BagToad merged commit afffc8e into cli:main Oct 8, 2024
17 of 19 checks passed
@BagToad BagToad changed the title create new func to avoid the bug on issue #61 Introduce safe host parsing function Oct 8, 2024
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.

Introduce safe host parsing command
4 participants