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

feature request: incorporate shellcheck for linting command blocks #191

Open
a-frantz opened this issue Sep 26, 2024 · 5 comments · May be fixed by #264
Open

feature request: incorporate shellcheck for linting command blocks #191

a-frantz opened this issue Sep 26, 2024 · 5 comments · May be fixed by #264
Labels
enhancement New feature or request

Comments

@a-frantz
Copy link
Member

This feature will be a complete rip-off from miniwdl. It's my favorite feature of miniwdl check and I want us to copy it.

For completeness: link to shellcheck site

It's been a while since I looked into this, but when I last looked there wasn't a good Rust crate for doing Bash linting. miniwdl shells out to shellcheck and parses the resulting STDOUT for a nice user interface. We are probably going to be stuck doing the same, which isn't ideal bc it's not very robust. But linting command blocks is a great feature I really want us to have, and it's worth getting our hands a little dirty IMO.

@thatRichman
Copy link

Very selfishly interested in this. I might be able to take a stab at it if this is still desired. Would I be correct in thinking this would be more appropriate for sprocket now?

@a-frantz
Copy link
Member Author

a-frantz commented Dec 6, 2024

We would certainly welcome that contribution! I've wanted this feature for a long time, but it hasn't made it up high enough in my priority list.

I think this code should live in the wdl-lint crate, not sprocket.

@claymcleod
Copy link
Member

Yeah I'd say this should live in wdl-lint as well. It's probably a "not on by default" lint that you have to opt in to. This will be quite a tricky one to implement gracefully—feel free to drop into the Zulip channel pointed to in the README.md to discuss with us!

@thatRichman
Copy link

Sure, would be happy to discuss! Looks like the linked Zulip channel is invite only, though.

@claymcleod
Copy link
Member

Oh, thanks for pointing that out! I've updated the link with the invitation code—could you try again?

@thatRichman thatRichman linked a pull request Dec 8, 2024 that will close this issue
12 tasks
thatRichman added a commit to thatRichman/wdl that referenced this issue Dec 8, 2024
Co-authored-by: Clay McLeod <3411613+claymcleod@users.noreply.github.com>
thatRichman added a commit to thatRichman/wdl that referenced this issue Dec 8, 2024
thatRichman added a commit to thatRichman/wdl that referenced this issue Dec 12, 2024
Co-authored-by: Clay McLeod <3411613+claymcleod@users.noreply.github.com>
thatRichman added a commit to thatRichman/wdl that referenced this issue Dec 18, 2024
Co-authored-by: Clay McLeod <3411613+claymcleod@users.noreply.github.com>
@a-frantz a-frantz linked a pull request Dec 18, 2024 that will close this issue
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants