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.

Sign up for GitHub

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

Jump to bottom

🐛 Fix release deletion #2

Merged
merged 3 commits into from
May 18, 2020
Merged

🐛 Fix release deletion #2

merged 3 commits into from
May 18, 2020

Conversation

bjeanes
Copy link
Contributor

@bjeanes bjeanes commented May 17, 2020 •
edited
Loading

With this change, release is correctly deleted:

Run bjeanes/delete-tag-and-release@fix-release-deletion
🏷  given tag is "unstable"
🍻  found 1 releases to delete
👍🏼  all releases deleted successfully!
✅  unstable deleted successfully!

GitHub converts a release into draft status when its underlying tag is deleted.

This means that the release deletion code ignored candidate releases for deletion
both because it was a draft (draft === false) and because it was no longer
associated with any tag.

Additionally, one of the parameters passed to fetch() was incorrect (url -> path) and its return type was misinterpreted.

Fixes #1

@bjeanes
🐛 Fix release deletion
47cd555 
GitHub converts a release into draft status when its underlying tag is deleted.

This means that the release deletion code ignored candidate releases for deletion
both because it was a draft (`draft === false`) and because it was no longer
associated with any tag.
@bjeanes
Fix usage of fetch() in release deletion
b0b5a69 
It seems like this might have been broken in a refactoring of sorts.
@dev-drprasad
Copy link
Owner

Thanks for catching the bug and PR!

Since deleteReleases and deleteTag are both async function, both API requests happen asynchronously. So, we may face race condition. Tag may get deleted before releases and releases will end up in draft state which is, this PR actually trying to solve. To fix this, you can await, deleteReleases func.

bjeanes
bjeanes commented May 18, 2020
View reviewed changes
index.js Outdated Show resolved Hide resolved
bjeanes
bjeanes commented May 18, 2020
View reviewed changes
index.js Outdated Show resolved Hide resolved
@bjeanes
Copy link
Contributor Author

bjeanes commented May 18, 2020

Good spot on that race condition! Thanks :)

@dev-drprasad dev-drprasad merged commit 53597e3 into dev-drprasad:master May 18, 2020
@bjeanes bjeanes deleted the fix-release-deletion branch May 18, 2020 14:58
bjeanes added a commit to bjeanes/indexer-sync that referenced this pull request May 18, 2020
@bjeanes
Switch off my action fork now PRs are merged
bb11e71 
dev-drprasad/delete-tag-and-release#2
dev-drprasad/delete-tag-and-release#3
@S-G0D S-G0D mentioned this pull request May 10, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dev-drprasad dev-drprasad dev-drprasad left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

It does not delete the release
2 participants
@bjeanes @dev-drprasad