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

Update jvm.sh to make shellcheck happier #272

Merged
merged 5 commits into from
Aug 9, 2023
Merged

Update jvm.sh to make shellcheck happier #272

merged 5 commits into from
Aug 9, 2023

Conversation

bf4
Copy link
Contributor

@bf4 bf4 commented Jul 25, 2023

Update jvm.sh to make shellcheck and people running set -eux happier

no unbound variables and set locals separately from assigning them

Update jvm.sh to make shellcheck and people running `set -eux` happier

no unbound variables and set locals separately from assigning them
@bf4 bf4 requested a review from a team as a code owner July 25, 2023 22:19
@Malax Malax added the skip changelog Pull requests that do not require changes to the CHANGELOG file label Jul 26, 2023
@Malax
Copy link
Member

Malax commented Jul 26, 2023

Hi @bf4!

Thanks for you contribution! Happy to merge this in after we get CI to pass!

We actually have ShellCheck in CI, which version did give you warnings about this? I ran CI for your PR and got formatter lint failures from shfmt - can you resolve those?

Copy link
Member

@Malax Malax left a comment

Choose a reason for hiding this comment

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

CI is failing

@bf4
Copy link
Contributor Author

bf4 commented Jul 27, 2023

Looking at the failures. Sorry for the delay.
It looks like the failures are just shfmt moving local foo; foo="" to local foo\nfoo="" which I'm okay with. I was just making a more minimal change.

lib/jvm.sh Show resolved Hide resolved
lib/jvm.sh Outdated Show resolved Hide resolved
bf4 added 2 commits July 27, 2023 21:30
```
shfmt -f . | grep -v "/vendor/" | xargs shfmt -i 2 -d | git apply
```
@Malax Malax self-assigned this Aug 7, 2023
@Malax Malax merged commit 0324809 into heroku:main Aug 9, 2023
9 checks passed
@Malax
Copy link
Member

Malax commented Aug 9, 2023

Thanks @bf4!

@bf4 bf4 deleted the patch-1 branch August 9, 2023 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip changelog Pull requests that do not require changes to the CHANGELOG file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants