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

More idiomatic way to fetch repository remote URL #1516

Merged
merged 2 commits into from
Jan 25, 2025
Merged

Conversation

o2sh
Copy link
Owner

@o2sh o2sh commented Jan 24, 2025

Tried to simplify the logic to get the repository remote URL

@o2sh o2sh added the refactor label Jan 24, 2025
@o2sh o2sh requested a review from Byron January 24, 2025 22:08
Copy link
Collaborator

@spenserblack spenserblack left a comment

Choose a reason for hiding this comment

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

LGTM! But, just to confirm, we're now changing from always falling back to an empty string to now distinguishing between "no remote found" vs "invalid remote found", right?

src/info/url.rs Outdated
Comment on lines 26 to 29
if let Some(url) = remote.url(gix::remote::Direction::Push) {
Ok(format_url(&url.to_string(), hide_token, http_url))
} else {
Ok(String::new())
Copy link
Collaborator

Choose a reason for hiding this comment

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

IDK if you have a preference, but this can also be written with Option::map and Option::unwrap_or_default I think.

Copy link
Owner Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

@spenserblack spenserblack left a comment

Choose a reason for hiding this comment

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

Oops, hit submit before switching to "approve"

Copy link
Collaborator

@Byron Byron left a comment

Choose a reason for hiding this comment

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

LGTM!

@o2sh o2sh merged commit e0fd7fa into main Jan 25, 2025
4 checks passed
@o2sh o2sh deleted the chore/repo-url-refactor branch January 25, 2025 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants