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

Shellcheck first pass #146

Merged
merged 4 commits into from
Feb 18, 2025

Conversation

gportay
Copy link
Contributor

@gportay gportay commented Feb 7, 2025

This is a first pass of shellcheck fixes.

It fixes the trivial errors and disables the more tricky ones for now.

@gportay
Copy link
Contributor Author

gportay commented Feb 14, 2025

What your opinion in making cqfd compliant to shellcheck?

@florentsfl
Copy link
Contributor

Yes I was wondering how much is that important since cqfd is meant to run with bash. Maybe @deribaucourt has an opinion on that ?

@gportay
Copy link
Contributor Author

gportay commented Feb 17, 2025

Yes I was wondering how much is that important since cqfd is meant to run with bash. Maybe @deribaucourt has an opinion on that ?

I really like using shellcheck, I learnt a lot from it in the past by making all my shell scripts compliant to shellcheck.

It solves lots of unexpected behaviors, especially concerning the globing or the preservation of whitspaces, and it helped me to write (way) better shell script.

So I strongly suggest to start using it.

@deribaucourt
Copy link
Member

I also think Shellcheck would be a great addition to the CI. Allows to catch minute bugs and improve code cleanliness

@gportay gportay force-pushed the shellcheck-first-pass branch from 0405e54 to 9f96324 Compare February 18, 2025 10:48
@deribaucourt
Copy link
Member

@gportay Can you rebase to fix conflicts? Thanks for your contributions!

@gportay
Copy link
Contributor Author

gportay commented Feb 18, 2025

@gportay Can you rebase to fix conflicts? Thanks for your contributions!

sure I will do.

The properties have their own global variable that are prefixed by their
section; every properties at the exception of custom_img_name (from
section project).

This adds and uses the new global variable project_custom_img_name.
This fixes the trivial shellcheck errors and ignores the others.
@gportay gportay force-pushed the shellcheck-first-pass branch from 9f96324 to 61a81ef Compare February 18, 2025 11:35
@florentsfl
Copy link
Contributor

Looks very nice

@florentsfl
Copy link
Contributor

should we merge that now or better wait for "cqfd 6" ?

@gportay
Copy link
Contributor Author

gportay commented Feb 18, 2025

Now is fine.

@florentsfl florentsfl merged commit 9970ebd into savoirfairelinux:master Feb 18, 2025
1 check passed
@gportay gportay deleted the shellcheck-first-pass branch February 18, 2025 16:19
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.

3 participants