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.

Sign up for GitHub

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

Jump to bottom

[CI][Dev] Add shell script linter #43080

Closed
kou opened this issue Jun 28, 2024 · 5 comments
Closed

[CI][Dev] Add shell script linter #43080

kou opened this issue Jun 28, 2024 · 5 comments
Assignees
@kou
Labels
Component: Continuous Integration Component: Developer Tools Type: enhancement
Milestone
19.0.0

Comments

@kou
Copy link
Member

kou commented Jun 28, 2024

Describe the enhancement requested

Candidate:

Component(s)

Continuous Integration, Developer Tools
@pitrou
Copy link
Member

pitrou commented Sep 16, 2024 •
edited
Loading

This sounds like a good idea to me. I'm very bad at writing shell scripts and I would appreciate this.

@kou
Copy link
Member Author

kou commented Sep 16, 2024

I've introduced shellcheck and shfmt to apache/arrow-go by apache/arrow-go@d486377#diff-63a9c44a44acf85fea213a857769990937107cf072831e1a26808cfde9d096b9 .
I'll do the same thing to apache/arrow after we completed the apache/arrow-go migration.

@pitrou
Copy link
Member

pitrou commented Nov 13, 2024

I still think this would be a good idea. Also cc @assignUser

@assignUser
Copy link
Member

Yeah bashls is screaming at me everytime I open one of our scripts xD

kou added a commit to kou/arrow that referenced this issue Nov 14, 2024
@kou
apacheGH-43080: [CI][Dev] Enable shellcheck
a3fe52f
@github-actions github-actions bot mentioned this issue Nov 14, 2024
Merged
assignUser pushed a commit that referenced this issue Nov 15, 2024
@kou
GH-43080: [CI][Dev] Enable shellcheck (#44724)
3b3af5d 
### Rationale for this change

shellcheck is a shell linter. It will find some potential problems in CI.

### What changes are included in this PR?

* Add a shellcheck entry to `.pre-commit-config.yaml`
* Enable shellcheck lint only for `ci/scripts/c_glib_build.sh`

We should enable shellcheck lint for all shell scripts eventually. 

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* GitHub Issue: #43080

Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Jacob Wujciak-Jens <jacob@wujciak.de>
@assignUser assignUser added this to the 19.0.0 milestone Nov 15, 2024
@assignUser
Copy link
Member

Issue resolved by pull request 44724
#44724

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kou kou

Labels
Component: Continuous Integration Component: Developer Tools Type: enhancement
Projects
None yet
Milestone
19.0.0
Development

No branches or pull requests
3 participants
@kou @pitrou @assignUser