Skip to content

ci: delete testing fork pre-releases in the upstream context#49

Merged
zimeg merged 2 commits intomainfrom
zimeg-ci-delete-fork-prereleases-3
Apr 18, 2025
Merged

ci: delete testing fork pre-releases in the upstream context#49
zimeg merged 2 commits intomainfrom
zimeg-ci-delete-fork-prereleases-3

Conversation

@zimeg
Copy link
Member

@zimeg zimeg commented Apr 18, 2025

Summary

This PR replaces the pull_request event with pull_request_target to delete pre-releases in the context of this upstream base branch.

Fixes an issue where deleting or closing forked PRs errored with an unauthorized token.

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 18, 2025
@zimeg zimeg added this to the Next Release milestone Apr 18, 2025
@zimeg zimeg self-assigned this Apr 18, 2025
@zimeg zimeg marked this pull request as ready for review April 18, 2025 00:05
@zimeg zimeg requested a review from a team as a code owner April 18, 2025 00:05
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.

📝 A note on the change for reviewers around!

# This workflow action deletes that pre-release when a PR is merged or closed.
on:
pull_request:
pull_request_target:
Copy link
Member Author

Choose a reason for hiding this comment

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

AFAICT this is the "same" event as pull_request but runs in the base branch context. That makes testing it within a PR not so simple I find 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Wow, good find!

suggestion: Always a good idea to leave a comment above around why we've using pull_request_target: otherwise a future maintainer may come in and change this back to pull_request:

@codecov
Copy link

codecov bot commented Apr 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.92%. Comparing base (e34e518) to head (598e01e).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #49   +/-   ##
=======================================
  Coverage   62.91%   62.92%           
=======================================
  Files         210      210           
  Lines       22147    22147           
=======================================
+ Hits        13933    13935    +2     
+ Misses       7128     7126    -2     
  Partials     1086     1086           

☔ 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.

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.

✅ LGTM! Left a minor suggestion but non-blocking to merge this PR when you're ready! It's exciting to see all of the pieces come together for fork support!

# This workflow action deletes that pre-release when a PR is merged or closed.
on:
pull_request:
pull_request_target:
Copy link
Member

Choose a reason for hiding this comment

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

Wow, good find!

suggestion: Always a good idea to leave a comment above around why we've using pull_request_target: otherwise a future maintainer may come in and change this back to pull_request:

@zimeg
Copy link
Member Author

zimeg commented Apr 18, 2025

@mwbrooks @hello-ashleyintech Thank y'all both for the reviews 🤠

Always a good idea to leave a comment

And also appreciate this callout! I added more to a comment in 598e01e on how branches are expected to be deleted for future inspection.

I'm also hopeful that this will be a final piece in the testing of PRs from forks for this time. Let's soon find out!

@zimeg zimeg merged commit 6dbc21e into main Apr 18, 2025
6 checks passed
@zimeg zimeg deleted the zimeg-ci-delete-fork-prereleases-3 branch April 18, 2025 17:53
@zimeg
Copy link
Member Author

zimeg commented Apr 18, 2025

📣 For this PR, created on an upstream branch, we find releases removed:

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

@zimeg
Copy link
Member Author

zimeg commented Apr 18, 2025

📣 For another PR, created on a fork, these releases are also deleted!

Identified pre-release tagname to 🔪: v3.0.4-pull-44-head
...
Successfully deleted v3.0.4-pull-44-head

https://github.com/slackapi/slack-cli/actions/runs/14539532571/job/40794613787

@zimeg
Copy link
Member Author

zimeg commented Apr 18, 2025

🫡 Removing pre-releases used for past testing that've since been merged:

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.

3 participants