Skip to content

fix(release): Make scripts exit on failure #105

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

NickLarsenNZ
Copy link
Member

@NickLarsenNZ NickLarsenNZ commented May 2, 2025

Fixes #103

If they were originally there to handle failures without exiting then that should be done through regular means without disabling the flag.

If they were originally there to handle failures without exiting then that should be done through regular means without disabling the flag.
@NickLarsenNZ NickLarsenNZ requested a review from adwk67 May 2, 2025 15:24
@NickLarsenNZ NickLarsenNZ self-assigned this May 2, 2025
@NickLarsenNZ NickLarsenNZ moved this to Development: Waiting for Review in Stackable Engineering May 2, 2025
@sbernauer sbernauer moved this from Development: Waiting for Review to Development: In Review in Stackable Engineering May 5, 2025
@@ -344,10 +344,7 @@ main() {
check_dependencies

# sanity checks before we start: folder, branches etc.
# deactivate -e so that piped commands can be used
set +e
Copy link
Member

Choose a reason for hiding this comment

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

This short-circuits without logging out helpful information. e.g. running this in main

./release/create-release-candidate-branch.sh -t 25.7.0-rc1 -w products

shows:

Expected release branch is missing: release-25.7

which is suppressed with this PR change, so I am not sure this is an improvement.

Copy link
Member

Choose a reason for hiding this comment

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

Although we certainly exit more consistently now, albeit with sometimes less info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Development: In Review
Development

Successfully merging this pull request may close these issues.

💥 Checks should be able to fail in release scripts (remove set +e)
2 participants