Skip to content

feat(build): add shfmt tool for shell formatting#565

Open
lengau wants to merge 7 commits into
mainfrom
work/add-shfmt
Open

feat(build): add shfmt tool for shell formatting#565
lengau wants to merge 7 commits into
mainfrom
work/add-shfmt

Conversation

@lengau

@lengau lengau commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Implement shell formatting tool (shfmt) as requested in #531.

Copilot AI review requested due to automatic review settings June 15, 2026 17:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds shfmt-based shell formatting to the project’s developer tooling, aligning with the request in issue #531 to standardize shell script formatting across the repository.

Changes:

  • Add make format-shell and include it in the top-level make format aggregator.
  • Add an install-shfmt Make target to help install shfmt via common package managers.
  • Add a pre-commit hook configuration for shfmt via pre-commit-shfmt.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
Makefile Extends the format target to include shell formatting.
common.mk Introduces format-shell and an install-shfmt helper target.
.pre-commit-config.yaml Adds a shfmt hook to run via pre-commit.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread common.mk Outdated
Comment thread .pre-commit-config.yaml
@lengau lengau force-pushed the work/add-shfmt branch 2 times, most recently from 7a9198e to ba0b03b Compare June 15, 2026 17:52
@lengau lengau requested a review from Copilot June 15, 2026 17:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread common.mk Outdated
Comment thread common.mk Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comment thread common.mk Outdated
Comment thread common.mk Outdated
Comment thread .pre-commit-config.yaml
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Alex Lowe <alex.lowe@canonical.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comment thread common.mk Outdated
Comment thread common.mk Outdated
Comment thread common.mk Outdated
@lengau lengau requested review from bepri and mr-cal June 17, 2026 19:03
Comment thread common.mk
format-shfmt: install-shfmt ##- Format shell scripts
@# jinja2 shell script templates are mistakenly counted as "true" shell scripts due to their shebang,
@# so explicitly filter them out
git ls-files -z | xargs -0 sh -c 'for f; do case "$$f" in *.sh.j2) continue;; esac; file --mime-type -Nn -- "$$f" | grep -q shellscript && printf "%s\0" "$$f"; done' -- | xargs -0 sh -c '[ $$# -eq 0 ] || exec shfmt -w "$$@"' --

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this calculation of the files to run on so much more complex than what existed in lint-shellcheck?

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.

4 participants