-
Notifications
You must be signed in to change notification settings - Fork 82
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
Improve publish-rhel
script [DI-343]
#823
Conversation
The action [failed](https://github.com/hazelcast/hazelcast-docker/actions/runs/11831672090/attempts/1) but without providing enough information to understand _why_. Changes: - Log the `get_image` function response (JSON) in case of failure - Improve failure annotation reporting (as per #821) - Address all Shellcheck warnings in case related - [Declare and assign separately to avoid masking return values](https://www.shellcheck.net/wiki/SC2155) - [Prefer putting braces around variable references even when not strictly required](https://www.shellcheck.net/wiki/SC2250) - [Quote the rhs of = in [[ ]] to prevent glob matchingQuote the rhs of = in [[ ]] to prevent glob matching](https://www.shellcheck.net/wiki/SC2053) _Partially addresses_ [DI-342](https://hazelcast.atlassian.net/browse/DI-342) Post-merge actions: - [ ] backport
publish-rhel
script [DI-342]publish-rhel
script [DI-343]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tested it?
I hadn't - because it's destructive I wanted to wait for a review. As it's already had one approval, I've run it here ✅. Unless you could think of a better way to test? |
.github/scripts/publish-rhel.sh
Outdated
set -o errexit -o nounset -o pipefail ${RUNNER_DEBUG:+-x} | ||
|
||
# shellcheck source=../.github/scripts/abstract-simple-smoke-test.sh | ||
. .github/scripts/abstract-simple-smoke-test.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the echoerr
function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's confusing that we have a test code in the production script. I would suggest either inline this function as it's very small or move it to another file echo.functions.sh
/log.functions.sh
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's confusing that we have a test code in the production script.
Good point.
I would suggest either inline this function as it's very small or move it to another file
echo.functions.sh
/log.functions.sh
?
For now I've duplicated it.
Ideally it should be externalised - but as hazelcast-packaging
directly references the script without any other context, a larger change is required to support that.
Backport of #823 The action [failed](https://github.com/hazelcast/hazelcast-docker/actions/runs/11831672090/attempts/1) but without providing enough information to understand _why_. Changes: - Log the `get_image` function response (JSON) in case of failure - Improve failure annotation reporting (as per #821) - Address all Shellcheck warnings in case related - [`Declare and assign separately to avoid masking return values`](https://www.shellcheck.net/wiki/SC2155) - [`Prefer putting braces around variable references even when not strictly required`](https://www.shellcheck.net/wiki/SC2250) - [`Quote the rhs of = in [[ ]] to prevent glob matchingQuote the rhs of = in [[ ]] to prevent glob matching`](https://www.shellcheck.net/wiki/SC2053) _Partially addresses_ [DI-343](https://hazelcast.atlassian.net/browse/DI-342) [DI-342]: https://hazelcast.atlassian.net/browse/DI-342?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [DI-343]: https://hazelcast.atlassian.net/browse/DI-343?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Backport of #823 The action [failed](https://github.com/hazelcast/hazelcast-docker/actions/runs/11831672090/attempts/1) but without providing enough information to understand _why_. Changes: - Log the `get_image` function response (JSON) in case of failure - Improve failure annotation reporting (as per #821) - Address all Shellcheck warnings in case related - [`Declare and assign separately to avoid masking return values`](https://www.shellcheck.net/wiki/SC2155) - [`Prefer putting braces around variable references even when not strictly required`](https://www.shellcheck.net/wiki/SC2250) - [`Quote the rhs of = in [[ ]] to prevent glob matchingQuote the rhs of = in [[ ]] to prevent glob matching`](https://www.shellcheck.net/wiki/SC2053) _Partially addresses_ [DI-343](https://hazelcast.atlassian.net/browse/DI-342) [DI-342]: https://hazelcast.atlassian.net/browse/DI-342?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [DI-343]: https://hazelcast.atlassian.net/browse/DI-343?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Backport of #823 The action [failed](https://github.com/hazelcast/hazelcast-docker/actions/runs/11831672090/attempts/1) but without providing enough information to understand _why_. Changes: - Log the `get_image` function response (JSON) in case of failure - Improve failure annotation reporting (as per #821) - Address all Shellcheck warnings in case related - [`Declare and assign separately to avoid masking return values`](https://www.shellcheck.net/wiki/SC2155) - [`Prefer putting braces around variable references even when not strictly required`](https://www.shellcheck.net/wiki/SC2250) - [`Quote the rhs of = in [[ ]] to prevent glob matchingQuote the rhs of = in [[ ]] to prevent glob matching`](https://www.shellcheck.net/wiki/SC2053) _Partially addresses_ [DI-343](https://hazelcast.atlassian.net/browse/DI-342) [DI-342]: https://hazelcast.atlassian.net/browse/DI-342?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [DI-343]: https://hazelcast.atlassian.net/browse/DI-343?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Currently is duplicated and should instead be centralised. [Raised from PR feedback](hazelcast/hazelcast-docker#823 (comment)). Relates to hazelcast/hazelcast-docker#842.
Currently is duplicated and should instead be centralised. [Raised from PR feedback](#823 (comment)). Requires hazelcast/hazelcast-packaging#251.
The action failed but without providing enough information to understand why.
Changes:
get_image
function response (JSON) in case of failureDeclare and assign separately to avoid masking return values
Prefer putting braces around variable references even when not strictly required
Quote the rhs of = in [[ ]] to prevent glob matchingQuote the rhs of = in [[ ]] to prevent glob matching
Partially addresses DI-343
Post-merge actions: