Skip to content

fix: compare only <owner>/<repo> when checking for rename#886

Merged
babblebey merged 18 commits intosemantic-release:masterfrom
jedwards1211:patch-1
Aug 14, 2024
Merged

fix: compare only <owner>/<repo> when checking for rename#886
babblebey merged 18 commits intosemantic-release:masterfrom
jedwards1211:patch-1

Conversation

@jedwards1211
Copy link
Contributor

@jedwards1211 jedwards1211 commented Jul 25, 2024 •
edited by hearts bot
Loading

This PR re-attempts the implementation made in #878 and reverted in #887, by staging a check in the plugin verifyConditions lifecycle using the owner/repo as opposed to the initial implementation where the context.options.repositoryUrl is compared against the fetched repo clone_url.

Related to #885

Screencast

Error Demo

screencast-bpconcjcammlapcogcnnelfmaeghhagj-2024.1.mp4

Success Demo

screencast-bpconcjcammlapcogcnnelfmaeghhagj-2024.2.mp4

@jedwards1211 jedwards1211 changed the title fix: ignore protocol mismatch when checking if repository is renamed fix: compare only <owner>/<repo> when checking for rename Jul 25, 2024
@gr2m gr2m mentioned this pull request Jul 26, 2024
Merged
babblebey
babblebey reviewed Jul 30, 2024
View reviewed changes
babblebey
babblebey reviewed Jul 30, 2024
View reviewed changes
babblebey
babblebey requested changes Jul 30, 2024
View reviewed changes
Copy link
Member

@babblebey babblebey left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great stuff @jedwards1211,

Do rake a look at the bit of nits and kindly resolve the conflict for now... We are looking to add some tests too... We'll look at how to go about that after this.

@jedwards1211
Copy link
Contributor Author

jedwards1211 commented Jul 31, 2024 •
edited by hearts bot
Loading

@babblebey okay I made those changes and included the current repositoryUrl and git_url values in the error message.

There are unrelated formatting changes in here now that I ran lint:prettier:fix on lib/definitions/errors.js -- is your CI not enforcing the code formatting, or did the formatting settings change?

@jedwards1211 jedwards1211 mentioned this pull request Jul 31, 2024
Closed
@jedwards1211
Copy link
Contributor Author

jedwards1211 commented Jul 31, 2024 •
edited by hearts bot
Loading

Okay I opened this issue: #890

@babblebey
Copy link
Member

babblebey commented Jul 31, 2024 •
edited by hearts bot
Loading

Run npm run lint:prettier:fix

@jedwards1211
Copy link
Contributor Author

jedwards1211 commented Jul 31, 2024 •
edited by hearts bot
Loading

sorry for the noise, vscode-prettier's fault

@jedwards1211
Copy link
Contributor Author

jedwards1211 commented Jul 31, 2024 •
edited by hearts bot
Loading

@babblebey okay I added tests for EMISMATCHGITHUBURL. I also noticed that the repo rename check was being skipped if running in a GitHub action, and that it was making a redundant request for the repo information. I was able to move things around to eliminate the redundant request, and check for rename even if running in a GitHub action.

babblebey
babblebey reviewed Jul 31, 2024
View reviewed changes
@jedwards1211
Copy link
Contributor Author

jedwards1211 commented Jul 31, 2024 •
edited by hearts bot
Loading

Nice testing setup y'all! I like the thoroughness of the GitHub mocking, and also the fact that you aren't using Jest lol

@jedwards1211
Copy link
Contributor Author

jedwards1211 commented Aug 2, 2024 •
edited by hearts bot
Loading

@babblebey anything else y'all need on this?

@jedwards1211 jedwards1211 mentioned this pull request Aug 2, 2024
Open
babblebey
babblebey reviewed Aug 2, 2024
View reviewed changes
babblebey
babblebey requested changes Aug 2, 2024
View reviewed changes
Copy link
Member

@babblebey babblebey left a comment

Choose a reason for hiding this comment

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

Hey @jedwards1211 😁,

I gave this a whirl 😉, changes looks great 👍.... and thank you for sticking to this. I have also gone on and written a description for the PR.

In order to fulfil the test coverage requirement mentioned in the reverted PR #887. Please take a look at the review comment.

@babblebey
Copy link
Member

babblebey commented Aug 5, 2024 •
edited by hearts bot
Loading

This is great stuff right here @jedwards1211. I commend you for sticking to this yea.

I have given another whirl and added a video demo of my testing to the PR Description (reviewers like that stuff 😉).

@babblebey babblebey requested a review from a team August 5, 2024 20:08
@jedwards1211
Copy link
Contributor Author

jedwards1211 commented Aug 6, 2024 •
edited by hearts bot
Loading

nice, thank you!

babblebey
babblebey approved these changes Aug 9, 2024
View reviewed changes
Copy link
Member

@babblebey babblebey left a comment

Choose a reason for hiding this comment

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

🚀

@babblebey babblebey merged commit 24ea2ee into semantic-release:master Aug 14, 2024
@github-actions
Copy link

github-actions bot commented Aug 14, 2024

🎉 This issue has been resolved in version 10.1.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@kc1r74p kc1r74p mentioned this pull request Aug 19, 2024
Closed
@oscard0m oscard0m mentioned this pull request Sep 13, 2024
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@babblebey babblebey babblebey approved these changes

Assignees

No one assigned

Labels

released

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jedwards1211 @babblebey