Skip to content

Conversation

@zimeg
Copy link
Member

@zimeg zimeg commented Apr 4, 2025

Summary

This PR fixes an issue where branch prereleases aren't deleted after a merge to main if the prerelease tag starts with a version from before the latest release.

Preview

In this example, we have multiple releases, a few stable tags, and some lingering releases. From most recent to least:

Which lines up with when the "build" branches were started - before v3.0.3 - and when these merged after v3.0.3.

The changes within check the last 6 stable releases for a matching branch name to decide which release to delete. This is bound to PRs that haven't gone "stale" while also avoiding perhaps unexpected deletes of long past releases. TBH that should never happen.

Notes

  • I'm now wondering if tests and similar uploaded artifacts encounter stranegness when the branch is worked on across versions, but I haven't seen anything so strange here 🔍

Requirements

@zimeg zimeg added semver:patch Use on pull requests to describe the release version increment build M-T: Changes to compilation and CI processes labels Apr 4, 2025
@zimeg zimeg added this to the Next Release milestone Apr 4, 2025
@zimeg zimeg self-assigned this Apr 4, 2025
@zimeg zimeg requested a review from a team as a code owner April 4, 2025 23:20
Copy link
Member Author

@zimeg zimeg left a comment

Choose a reason for hiding this comment

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

📝 Leaving a note on the updated command used, but am avoiding much detail in the logic below for the sake of reviewing!

echo "Identified pre-release tagname to 🔪: $TAG_NAME"
# Delete the pre-release
GH_DEBUG=1 gh release --repo slackapi/slack-cli delete $TAG_NAME -y --cleanup-tag
RELEASES=$(gh release list --repo="slackapi/slack-cli" --order="desc" --json="tagName" --exclude-drafts --exclude-pre-releases --limit=6 --jq ".[] | .tagName")
Copy link
Member Author

Choose a reason for hiding this comment

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

This command is similar to the one before, but now lists more recent releases:

$ gh release list --repo="slackapi/slack-cli" --order="desc" --json="tagName" --exclude-drafts --exclude-pre-releases --limit=6 --jq ".[] | .tagName"
v3.0.4
v3.0.3
v3.0.2

Copy link
Member

Choose a reason for hiding this comment

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

Awesome find! Any reason why you chose --limit 6? It looks like the default is --limit 30 which could prevent undeleted pre-releases further back?

Copy link
Member Author

Choose a reason for hiding this comment

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

@mwbrooks Hmm I'm glad you called this out! 🤔

I agree 6 seems to be not enough releases, but I also wanted to limit this to avoid rate limits in the following loop if the branch isn't found.

That might have a better workaround though...

Keeping some --limit feels right to me since the default isn't clear otherwise, and this is now 24 which should cover more branches!

@codecov
Copy link

codecov bot commented Apr 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.79%. Comparing base (3c995d5) to head (9ab440a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #15      +/-   ##
==========================================
- Coverage   62.81%   62.79%   -0.02%     
==========================================
  Files         210      210              
  Lines       22053    22053              
==========================================
- Hits        13852    13848       -4     
- Misses       7121     7127       +6     
+ Partials     1080     1078       -2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zimeg zimeg requested a review from hello-ashleyintech April 4, 2025 23:22
@zimeg
Copy link
Member Author

zimeg commented Apr 4, 2025

📣 If these changes seem alright and this merges, I will clean up the existing prereleases that should've been deleted.

I notice theses appear here:
https://github.com/slackapi/slack-cli/actions/workflows/delete-pr-build-on-close.yml?query=is%3Afailure

Copy link
Contributor

@hello-ashleyintech hello-ashleyintech left a comment

Choose a reason for hiding this comment

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

🚀

Copy link
Member

@mwbrooks mwbrooks left a comment

Choose a reason for hiding this comment

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

✅ Absolutely incredible find @zimeg! I can't imagine how difficult it was to track down the issue to old releases 👏🏻

✏️ Left a minor question, but non-blocking to merging this PR!

echo "Identified pre-release tagname to 🔪: $TAG_NAME"
# Delete the pre-release
GH_DEBUG=1 gh release --repo slackapi/slack-cli delete $TAG_NAME -y --cleanup-tag
RELEASES=$(gh release list --repo="slackapi/slack-cli" --order="desc" --json="tagName" --exclude-drafts --exclude-pre-releases --limit=6 --jq ".[] | .tagName")
Copy link
Member

Choose a reason for hiding this comment

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

Awesome find! Any reason why you chose --limit 6? It looks like the default is --limit 30 which could prevent undeleted pre-releases further back?

@zimeg
Copy link
Member Author

zimeg commented Apr 7, 2025

@hello-ashleyintech @mwbrooks Thank y'all both for the reviews and feedback 👾 ✨

Once tests are passing I will merge this and delete past pre-releases if these changes delete this pre-release as expected!

@zimeg zimeg merged commit 584a055 into main Apr 7, 2025
5 checks passed
@zimeg zimeg deleted the zimeg-ci-delete-versioned-prereleases branch April 7, 2025 23:11
@zimeg
Copy link
Member Author

zimeg commented Apr 7, 2025

🪓 https://github.com/slackapi/slack-cli/actions/runs/14321328571/job/40138683904

Identified pre-release tagname to 🔪: v3.0.4-zimeg-ci-delete-versioned-prereleases
...
Successfully deleted v3.0.4-zimeg-ci-delete-versioned-prereleases

@zimeg
Copy link
Member Author

zimeg commented Apr 7, 2025

📝 Deleting prereleases causes the workflow to fail with the following:

Identified pre-release tagname to 🔪: v3.0.4-v3.0.2-zimeg-build-dev
...
Failed to find v3.0.4-v3.0.2-zimeg-build-dev, trying next...
...

But this seems specific to these manual deletions and not merges otherwise? I'll keep watch of this and follow up as needed 🫡

Edit: https://github.com/slackapi/slack-cli/actions/runs/14321530256/job/40139248428

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build M-T: Changes to compilation and CI processes semver:patch Use on pull requests to describe the release version increment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants