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

dist/tools/buildsystem_sanity_check: make shellcheck happy #20721

Conversation

maribu
Copy link
Member

@maribu maribu commented Jun 4, 2024

Contribution description

The build system sanity check test script currently does not pass shellcheck, preventing PRs improving it from getting merged.

This commit fixes the issues pointed out by shellcheck and should not have any functional changes, except for the github annotation teardown now being reached after the call to main.

Testing procedure

The test script should work as before.

Issues/PRs references

None

@maribu maribu added Area: build system Area: Build system Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 4, 2024
@github-actions github-actions bot added Area: tools Area: Supplementary tools and removed Area: build system Area: Build system labels Jun 4, 2024
@riot-ci
Copy link

riot-ci commented Jun 4, 2024

Murdock results

✔️ PASSED

098167a dist/tools/buildsystem_sanity_check: fix and cleanup

Success Failures Total Runtime
1 0 1 01m:15s

Artifacts

@maribu maribu force-pushed the dist/tools/buildsystem_sanity_check/make-shellcheck-happy branch from 0b47100 to 10afe6f Compare June 4, 2024 11:39
@maribu maribu marked this pull request as draft June 4, 2024 11:46
@maribu
Copy link
Member Author

maribu commented Jun 4, 2024

It looks like Github annotation is broken, and likely was never working here. I'll try to fix this as well first and mark this as ready when it works.

@maribu maribu force-pushed the dist/tools/buildsystem_sanity_check/make-shellcheck-happy branch from 10afe6f to 28c3296 Compare June 4, 2024 20:43
@maribu maribu marked this pull request as ready for review June 4, 2024 20:44
Copy link
Contributor

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

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

LGTM, but mind that I'm not really an expert on bash scripts. Therefore only a soft-ACK.

dist/tools/buildsystem_sanity_check/check.sh Outdated Show resolved Hide resolved
@maribu
Copy link
Member Author

maribu commented Jun 7, 2024

Maybe @miri64 could give this a second look?

The github annotations were tested in #20472 (comment) (that is a warning that is not part of this PR, but that warning is generated via the error_with_message() function (as do all other functions in this file), so the infrastructure indeed should work now).

@maribu maribu force-pushed the dist/tools/buildsystem_sanity_check/make-shellcheck-happy branch from 231861a to 05d3303 Compare June 7, 2024 09:44
@maribu maribu force-pushed the dist/tools/buildsystem_sanity_check/make-shellcheck-happy branch from 05d3303 to c282e5a Compare June 7, 2024 11:50
@maribu
Copy link
Member Author

maribu commented Jun 7, 2024

All green, just needs an ACK 😉

The build system sanity check test script currently does not pass
shellcheck, preventing PRs improving it from getting merged.

In addition, the github annotation was broken.

This commit fixes the issues pointed out by shellcheck as well as
the Github annotation.

Co-authored-by: mguetschow <mikolai.guetschow@tu-dresden.de>
@maribu maribu force-pushed the dist/tools/buildsystem_sanity_check/make-shellcheck-happy branch from c282e5a to 098167a Compare June 7, 2024 18:12
@maribu maribu enabled auto-merge June 7, 2024 18:13
@maribu maribu added this pull request to the merge queue Jun 7, 2024
Merged via the queue into RIOT-OS:master with commit ce8f89b Jun 7, 2024
26 checks passed
@maribu maribu deleted the dist/tools/buildsystem_sanity_check/make-shellcheck-happy branch June 8, 2024 00:08
@maribu
Copy link
Member Author

maribu commented Jun 8, 2024

Thx :)

@mguetschow mguetschow added this to the Release 2024.07 milestone Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants