Skip to content
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

github action: enforce issue detection with bash #8168

Merged
merged 5 commits into from
Jun 24, 2021

Conversation

d-a-v
Copy link
Collaborator

@d-a-v d-a-v commented Jun 23, 2021

Per discussion with @mcspr.

When not specifiying bash as running shell, default options do not include -o pipefail as it should.
It may not prevent a script to stop when one component of a pipe chain fails (ex: false | true won't fail but it should).
As a consequence in this PR, bash is explicitely specified as default shell.

Also enforcing issue detection by breaking down commands
echo "$(command)" >> file => var=$(command) then echo ${var} >> file

@@ -25,7 +28,8 @@ jobs:
- name: Set GIT tag name
run: |
# Sets an environment variable used in the next steps
echo "TRAVIS_TAG=$(git describe --exact-match --tags)" >> $GITHUB_ENV
TRAVIS_TAG=$(git describe --exact-match --tags)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
TRAVIS_TAG=$(git describe --exact-match --tags)"
TRAVIS_TAG=$(git describe --exact-match --tags)

(I typed this wrong)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for reviewing (I didn't make a copy-paste, It's simply that I made the same mistake)

Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

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

Silly GH CI bug, good catch.

@d-a-v d-a-v added this to the 3.0.1 milestone Jun 24, 2021
@earlephilhower earlephilhower merged commit 04c2322 into esp8266:master Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants