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

Autoformat all bash scripts #42

Merged
merged 5 commits into from
Nov 6, 2024

Conversation

simonbaird
Copy link
Collaborator

@simonbaird simonbaird commented Nov 5, 2024

Not sure if this will be popular or not, but see what you think about it.

I'd be happy to tweak the indenting rules (tabs, 2 spaces, whatever), so let me know if there's a preference.

@simonbaird simonbaird force-pushed the autoformat-scripts branch 3 times, most recently from 25e24af to a9ee375 Compare November 5, 2024 22:52
I want to use this in the next commit as a way to identify the bash
scripts in the hacks directory. (Since they're not all ending with
.sh we can't use the filename extention to identify them.)
Add Makefile targets to install shfmt and run it.

shfmt comes from https://github.com/mvdan/sh
The changes in this commit were made by running `make format` and
checking in all the changes produced.
Prevent merging a PR if the bash hasn't been consistently formatted
using `make format`.
@simonbaird simonbaird marked this pull request as ready for review November 5, 2024 23:27
Copy link
Member

@chmeliik chmeliik left a comment

Choose a reason for hiding this comment

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

Consistent formatting is good 👍

Copy link
Member

@jduimovich jduimovich left a comment

Choose a reason for hiding this comment

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

LGTM
The env-template.sh workaround can later be fixed by moving it out of the rhtap directory.
It was only there when this repo was used "live" ... As its should not be there we end up having other places where we deal with it specially.
It was also supposed to be "generated" in future, when we add metadata ot the scripts which can be used to define parameters (for tekton for example)

@jduimovich jduimovich merged commit 767a3a0 into redhat-appstudio:main Nov 6, 2024
2 checks passed
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