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

Add bashate to CI #417

Merged
merged 6 commits into from
Feb 19, 2025
Merged

Add bashate to CI #417

merged 6 commits into from
Feb 19, 2025

Conversation

markparonyan
Copy link
Contributor

ref #411

@markparonyan
Copy link
Contributor Author

markparonyan commented Feb 12, 2025

@yegor256 Quoting bashate -i E006,E003 "$(find . -name '*.sh')" passes all the filenames as one argument, which is not what bashate expects. But our shellcheck requires it. Should we # shellcheck disable=SC2046?

@yegor256
Copy link
Owner

yegor256 commented Feb 13, 2025

@markparonyan you should put all names into an array and then use the array as "${files[@]}", see https://www.shellcheck.net/wiki/SC2046

Just imagine, what will happen if you do this bashate -i E006,E003 $(find . -name '*.sh') and one of the files has this name: a b c d.sh

@markparonyan
Copy link
Contributor Author

markparonyan commented Feb 13, 2025

@yegor256 Thanks! I fixed that. Should I also refactor bash scripts across the project according to bashate?
Also what are the issue with this two ci jobs: make / make and make / make-macos? I see them failing in every PR. How can I fix it, so system will allow merging?

@yegor256
Copy link
Owner

@markparonyan yes, of course you have to fix all bashate warnings before we can merge. Other issues -- make a different ticket about them, we'll see what we can do.

@markparonyan
Copy link
Contributor Author

@yegor256 please check

@yegor256 yegor256 merged commit 68ed9d7 into yegor256:master Feb 19, 2025
12 of 14 checks passed
@yegor256
Copy link
Owner

@markparonyan thanks a lot!

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.

2 participants