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

test(git): find upstream remote when using ssh #926

Merged
merged 3 commits into from
Oct 21, 2024
Merged

Conversation

kemitix
Copy link
Contributor

@kemitix kemitix commented Oct 19, 2024

Description

When looking for the owner and repo of the remote for the current HEAD, we will now try to parse the SSH format of a remote if it fails to match an HTTPS format of a remote.

Motivation and Context

When the git-cliff repo is cloned for development using the SSH protocol, the test repo::test::git_upstream_remote fails.

The upstream_remote function was relying on url::Url::parse to extract the owner and repo from the url. But that only works when the repo is cloned using HTTPS, e.g. https://github.com/orhun/git-cliff.git. However, this would fail to parse when cloned using SSH, e.g. git@github.com:orhun/git-cliff.git.

Now, if the url::URL::parser fails, we now try to parse an SSH remote in the format git@hostname:owner/repo.git.

The error from upstream_remote also notes that a possible reason for it failing would be that the HEAD is detached.

How Has This Been Tested?

  • Fork the orhun/git-cliff repo as kemitix/git-cliff
  • Clone using SSH, with a remote of git@github.com:kemitix/git-cliff.git
  • Run cargo test - all tests pass, including repo::test::git_upstream_remote

Screenshots / Logs (if applicable)

n/a

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (no code change)
  • Refactor (refactoring production code)
  • Other

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.
  • I have formatted the code with rustfmt.
  • I checked the lints with clippy.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@kemitix kemitix requested a review from orhun as a code owner October 19, 2024 15:07
Copy link

welcome bot commented Oct 19, 2024

Thanks for opening this pull request! Please check out our contributing guidelines! ⛰️

@codecov-commenter
Copy link

codecov-commenter commented Oct 19, 2024

Codecov Report

Attention: Patch coverage is 74.07407% with 7 lines in your changes missing coverage. Please review.

Project coverage is 42.42%. Comparing base (82b10ac) to head (ea50194).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
git-cliff-core/src/repo.rs 74.08% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #926      +/-   ##
==========================================
+ Coverage   41.72%   42.42%   +0.71%     
==========================================
  Files          21       21              
  Lines        1678     1695      +17     
==========================================
+ Hits          700      719      +19     
+ Misses        978      976       -2     
Flag Coverage Δ
unit-tests 42.42% <74.08%> (+0.71%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Owner

@orhun orhun left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good! I had a few comments regarding refactoring :)

git-cliff-core/src/repo.rs Outdated Show resolved Hide resolved
git-cliff-core/src/repo.rs Outdated Show resolved Hide resolved
git-cliff-core/src/repo.rs Outdated Show resolved Hide resolved
git-cliff-core/src/repo.rs Outdated Show resolved Hide resolved
@kemitix kemitix force-pushed the ssh-remotes branch 2 times, most recently from 2ea058f to 87fc703 Compare October 21, 2024 07:57
The `upstream_remote` function was relying on `url::Url::parse` to extract the `owner` and `repo` from the `url`. But that only works when the repo is cloned using a URL, e.g. `https://github.com/orhun/git-cliff.git`. However, this would fail to parse when cloned using SSH, e.g. `git@github.com:orhun/git-cliff.git`.

If the url::URL::parser fails, we now try to parse an SSH remote in the format `git@hostname:owner/repo.git`.

The error from `upstream_remote` also notes that a posible reason for it failing would be that the `HEAD` is detached.
git-cliff-core/src/repo.rs Outdated Show resolved Hide resolved
git-cliff-core/src/repo.rs Outdated Show resolved Hide resolved
@orhun
Copy link
Owner

orhun commented Oct 21, 2024

Thanks for the PR!

@orhun orhun changed the title feat: find upstream remote when using ssh test(git): find upstream remote when using ssh Oct 21, 2024
@orhun orhun merged commit 2e65a72 into orhun:main Oct 21, 2024
63 of 65 checks passed
Copy link

welcome bot commented Oct 21, 2024

Congrats on merging your first pull request! ⛰️

@kemitix kemitix deleted the ssh-remotes branch October 22, 2024 06:35
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.

3 participants