chore: add shellcheck and shfmt for scripts#520
chore: add shellcheck and shfmt for scripts#520vprashar2929 wants to merge 2 commits intoopenshift:mainfrom
Conversation
This commit adds support for running shellcheck and shfmt on the must-gather scripts. Changes include: - Add lint and fmt targets to Makefile for running shellcheck and shfmt - Add hack/tools.sh script to install shellcheck and shfmt binaries - Add hack/utils.bash with helper functions for tool installation - Fix shellcheck warnings in common.sh: - Remove useless echo in variable assignments (SC2116) - Add shellcheck disable for variables used by sourcing scripts (SC2034) - Add shellcheck disable for sed commands (SC2001) - Reformat common.sh and monitoring_common.sh with shfmt - Add /tmp to .gitignore for local tool binaries Signed-off-by: vprashar2929 <vibhu.sharma2929@gmail.com>
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: vprashar2929 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
We can run these make targets in our existing CI's: https://github.com/openshift/release/blob/master/ci-operator/config/openshift/must-gather/openshift-must-gather-main.yaml |
|
@vprashar2929 this looks great. It's an important start. +1 for adding the tooling here. Before it's ok to start updating (even just formatting) the scripts I'd like to first spend some time producing unit tests. So we can reduce a risk of regressions or accidentally changing a code that can lose it's ability to be understood by the original author.
|
|
I agree and I am happy to hold this off until I add tests to ensure that all must gather scripts are validated against cluster and provide the expected results |
Signed-off-by: vprashar2929 <vibhu.sharma2929@gmail.com>
d7448f3 to
e3e8985
Compare
|
@vprashar2929: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This commit adds support for running shellcheck and shfmt on the must-gather scripts.
Changes include: