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

[ci] Add shellcheck linter #17279

Closed
mx-psi opened this issue Dec 28, 2022 · 10 comments
Closed

[ci] Add shellcheck linter #17279

mx-psi opened this issue Dec 28, 2022 · 10 comments
Labels
ci-cd CI, CD, testing, build issues github_actions Pull requests that update Github_actions code good first issue Good for newcomers never stale Issues marked with this label will be never staled and automatically removed priority:p2 Medium

Comments

@mx-psi
Copy link
Member

mx-psi commented Dec 28, 2022

Component(s)

No response

Describe the issue you're reporting

shellcheck is a static analysis tool for identifying issues on shell scripts. It could be useful for identifying issues like #17037 and it's generally good for enforcing idiomatic and sensible shell scripts.

Running something like:

shellcheck -x -e SC2059 -e SC2086 .github/workflows/scripts/*.sh internal/buildscripts/*.sh

brings up about ~15 issues to be addressed today.

To ensure we continously lint our scripts with shellcheck, we should add a Github workflow that lints our shell scripts with shellcheck and fixes the issues pointed out by the command above.

@mx-psi mx-psi added good first issue Good for newcomers priority:p2 Medium ci-cd CI, CD, testing, build issues labels Dec 28, 2022
@github-actions
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@mohitsaxenaknoldus
Copy link

@mx-psi @jpkrohling PR: #17292

@github-actions
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

@github-actions github-actions bot added the Stale label Feb 28, 2023
@mx-psi mx-psi removed the Stale label Mar 1, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 1, 2023

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

  • issue: Github issue template generation code needs this to generate the corresponding labels.

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the Stale label Jul 3, 2023
@mx-psi mx-psi removed the Stale label Jul 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2023

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

  • issue: Github issue template generation code needs this to generate the corresponding labels.

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the Stale label Sep 4, 2023
@mx-psi mx-psi removed the Stale label Sep 4, 2023
Copy link
Contributor

github-actions bot commented Nov 6, 2023

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

  • issue: Github issue template generation code needs this to generate the corresponding labels.

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the Stale label Nov 6, 2023
@mx-psi mx-psi added never stale Issues marked with this label will be never staled and automatically removed and removed Stale labels Nov 6, 2023
@jpkrohling
Copy link
Member

@mohitsaxenaknoldus, are you still interested in working on this one?

evan-bradley added a commit that referenced this issue Apr 23, 2024
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
So i fixed the issues which shellcheck displayed, which were only 3
double quote issues and the `read -r` issue.
After looking into the closed PR i also replaced `printf`-statements
with `echo`.

For the shellcheck-worfklow I used
[https://github.com/ludeeus/action-shellcheck](url)
I didn't find a way to check multiple directories at once, so we just
use 2 steps here.

I added the disabled checks provided by the intentional issue, but i'm
not quite sure if we need to add `-x` here. Couldn't find anything in
the documentation so far.

On top of that i'm not quite sure if we should run the shellcheck in
dependency of any other workflows or just on its own.

Edit:
I added the option `ignore_paths` so that shellcheck-action is only run
in specified paths and ignores subpaths of this directory.

**Link to tracking Issue:** <Issue number if applicable>
- #17279 

**Testing:** <Describe what testing was performed and which tests were
added.>

**Documentation:** <Describe the documentation added.>

---------

Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com>
@led0nk
Copy link
Contributor

led0nk commented Apr 24, 2024

resolved by #32052

@mx-psi
Copy link
Member Author

mx-psi commented Apr 24, 2024

Thank you @led0nk!

@mx-psi mx-psi closed this as completed Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-cd CI, CD, testing, build issues github_actions Pull requests that update Github_actions code good first issue Good for newcomers never stale Issues marked with this label will be never staled and automatically removed priority:p2 Medium
Projects
None yet
Development

No branches or pull requests

4 participants