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

docker: fix dockerfile warnings #4080

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

jaydee-coder
Copy link
Contributor

@jaydee-coder jaydee-coder commented Nov 4, 2024

The following warnings were emitted when running make docker-test:

 2 warnings found (use docker --debug to expand):
 - LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 11)
 - JSONArgsRecommended: JSON arguments recommended for CMD to prevent unintended behavior related to OS signals (line 12)

This change fixes both of these 2 trivial warnings.

I added the bash -i option just in case some of the tests/tools care about whether the shell is an interactive shell or not.
Even though the tests passed successfully without that flag, I thought I'd add it just in case. It only added 0.5s to the total test time (64.8s -> 65.3s) so it's probably good to keep. Please let me know if you'd want me to remove it.

The following warnings were emitted when running `make docker-test`:
```
 2 warnings found (use docker --debug to expand):
 - LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 11)
 - JSONArgsRecommended: JSON arguments recommended for CMD to prevent unintended behavior related to OS signals (line 12)
```

This change fixes both of these 2 trivial warnings.
@jaydee-coder
Copy link
Contributor Author

Actually timed it again, and now the tests take 63.2s, so the change is in the noise and probably doesn't affect timing.

@junegunn junegunn merged commit 6816b7d into junegunn:master Nov 4, 2024
5 checks passed
@junegunn
Copy link
Owner

junegunn commented Nov 4, 2024

Thanks!

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