Skip to content

Conversation

dannywillems
Copy link
Member

@dannywillems dannywillems commented Jul 25, 2025

Fixing the lint issues now.

@dannywillems dannywillems enabled auto-merge July 25, 2025 10:06
@dannywillems dannywillems requested a review from volhovm July 25, 2025 10:18
@find . -name "*.sh" \
-not -path "*/target/*" \
-not -path "*/node_modules/*" \
-not -path "*/website/docs/developers/scripts/setup/*" \
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scripts we provide to the developers should be ignored as they do not to be executables, and shellcheck will complain.

Replace exit code checking pattern from '\! cmd' to using variables for
better code clarity. Wrap long command lines at 80 characters and ensure
proper variable quoting to comply with shellcheck recommendations.

- Use curl_exit_code, sed_exit_code, tar_exit_code variables
- Fix SC2181 violations by avoiding $? checks
- Fix SC2086 violations by adding quotes around variables
- Improve readability with proper line wrapping
cargo clippy --all-targets -- -D warnings --allow clippy::mutable_key_type

.PHONY: lint-bash
lint-bash: ## Check all shell scripts using shellcheck
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works for me.

Should we also add shellcheck into install-system-deps.sh? I'd definitely add it to the flake.nix since it's something that developers might want to run locally.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point for the doc!

@dannywillems dannywillems merged commit 808144d into develop Jul 25, 2025
69 of 70 checks passed
@dannywillems dannywillems deleted the dw/add-shellcheck branch October 16, 2025 11:52
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