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

Minor script beautifying #819

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alessfg
Copy link
Contributor

@alessfg alessfg commented Jul 20, 2023

* Fix small bug with using the -n flag instead of the -f flag when checking if files are available (https://github.com/nginxinc/docker-nginx/blob/master/Dockerfile-alpine-slim.template#L76-L78)
* Fix some shell linter warnings
* Minor script formatting/beautifying

Rebased this PR to only include some minor script formatting

@alessfg alessfg marked this pull request as draft July 20, 2023 22:29
@alessfg alessfg force-pushed the bug-fix-and-linter branch 5 times, most recently from d618fd4 to 0052b8b Compare July 20, 2023 23:52
@alessfg alessfg marked this pull request as ready for review July 24, 2023 20:25
@thresheek
Copy link
Collaborator

Hello!

Can you split actual fixes (like -n vs -f) and unrelated changes into separate commits?

Also, I'm not sure whitespace (and beautyfing, like splitting oneliners into multilines) changes are worth the effort, since by default vim will not honor modelines anyway - and they would pollute the history and blame.

@alessfg
Copy link
Contributor Author

alessfg commented Aug 2, 2023

See #824 😄

@alessfg alessfg force-pushed the bug-fix-and-linter branch 2 times, most recently from bdc3c7e to 5f213fc Compare August 3, 2023 13:22
@alessfg alessfg changed the title Correctly check if files are available in Alpine Linux, address some shell linter warnings, and do some minor script beautifying Minor script beautifying Aug 7, 2023
@alessfg
Copy link
Contributor Author

alessfg commented Aug 11, 2023

I do think there is something to be gained in readability for folks that are looking through the scripts either here or using a code editor instead of VIM, but I get your point. I will tweak the PR to get rid of some of the superfluous "beautifying" (e.g. revert the oneliner to multiline changes).

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