Skip to content

Conversation

@zimeg
Copy link
Member

@zimeg zimeg commented Apr 14, 2025

Summary

This PR escapes the branch name of pull requests for use in tests started with #36.

Preview

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 14, 2025
@zimeg zimeg added this to the Next Release milestone Apr 14, 2025
@zimeg zimeg self-assigned this Apr 14, 2025
@codecov
Copy link

codecov bot commented Apr 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.91%. Comparing base (9e6bc44) to head (fa32ad4).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #38      +/-   ##
==========================================
- Coverage   62.92%   62.91%   -0.02%     
==========================================
  Files         210      210              
  Lines       22149    22149              
==========================================
- Hits        13937    13934       -3     
- Misses       7126     7129       +3     
  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.

@zimeg
Copy link
Member Author

zimeg commented Apr 14, 2025

📝 Since this PR is created on a forked branch, the required "build-lint-test-e2e-test" workflow does not start. Changes in #36 let us start this after a review:

wip

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 few notes on these changes for the kind reviewer while tests run:

Comment on lines +270 to +271
BRANCH_NAME=$(echo "<< parameters.release_ref >>" | sed 's/\//-/g')
RELEASE_REF="${LAST_SEMVER_TAG}-${BRANCH_NAME}"
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 replaces / with - as if the slash stretches between words. This makes a valid semantic version.

Copy link
Member

Choose a reason for hiding this comment

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

LOVE THIS! ❤️

Comment on lines +390 to +395
if [[ -z "$CIRCLE_BRANCH" || "$CIRCLE_BRANCH" == pull/* ]]; then
BRANCH_NAME="main"
echo "Performing the standard end-to-end test suite for changes of a forked branch"
else
BRANCH_NAME="$CIRCLE_BRANCH"
fi
Copy link
Member Author

Choose a reason for hiding this comment

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

📝 Changes the E2E tests are not with this repo so we are defaulting to the main branch.

This is required since matching PR numbers might or might not exist upstream which can cause unexpected testing behavior!

Copy link
Member Author

Choose a reason for hiding this comment

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

📝 I am making a note that this is worth mentioning in the changes of #36!

@zimeg zimeg marked this pull request as ready for review April 14, 2025 22:59
@zimeg zimeg requested a review from a team as a code owner April 14, 2025 22:59
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.

👏🏻 Great work @zimeg!

✅ LGTM let's merge this and try it out!

Comment on lines +270 to +271
BRANCH_NAME=$(echo "<< parameters.release_ref >>" | sed 's/\//-/g')
RELEASE_REF="${LAST_SEMVER_TAG}-${BRANCH_NAME}"
Copy link
Member

Choose a reason for hiding this comment

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

LOVE THIS! ❤️

@zimeg
Copy link
Member Author

zimeg commented Apr 14, 2025

@mwbrooks Thanks so much! I'm hoping this'll make prereleases a bit-more-pleasant-to-read 👾

@zimeg zimeg merged commit f23058b into slackapi:main Apr 14, 2025
6 checks passed
@zimeg zimeg deleted the ci-test-forks-branch branch April 14, 2025 23:07
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.

2 participants