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: Improve Docker examples readme #17894

Merged
merged 2 commits into from
Nov 26, 2024
Merged

Conversation

fvaleri
Copy link
Contributor

@fvaleri fvaleri commented Nov 21, 2024

This patch adds some clarity about the environment variables usage and limitations. It also removes the invalid example and adds few style fixes.

See #17705 (comment).

This patch adds some clarity about the environment variables usage and limitations.
It also removes the invalid example and adds few style fixes.

Signed-off-by: Federico Valeri <[email protected]>
@github-actions github-actions bot added docs docker Official Docker image labels Nov 21, 2024
@fvaleri
Copy link
Contributor Author

fvaleri commented Nov 21, 2024

@VedarthConfluent @kagarwal06 fyi

@fvaleri
Copy link
Contributor Author

fvaleri commented Nov 25, 2024

@showuon @mimaison fyi

Copy link
Contributor

@showuon showuon left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Left some comments.

docker/examples/README.md Show resolved Hide resolved
docker/examples/README.md Outdated Show resolved Hide resolved
docker/examples/README.md Outdated Show resolved Hide resolved
@showuon
Copy link
Contributor

showuon commented Nov 25, 2024

@VedarthConfluent @kagarwal06, call for review. Thanks.

Copy link
Contributor

@VedarthConfluent VedarthConfluent left a comment

Choose a reason for hiding this comment

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

Thanks for updating the docs for better clarity! LGTM.

Signed-off-by: Federico Valeri <[email protected]>
@mimaison
Copy link
Member

Why are we keeping this information in a README, shouldn't this be directly part of the documentation?

Copy link
Contributor

@showuon showuon left a comment

Choose a reason for hiding this comment

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

LGTM!

@showuon
Copy link
Contributor

showuon commented Nov 26, 2024

Why are we keeping this information in a README, shouldn't this be directly part of the documentation?

+1. @fvaleri , could you help create a JIRA and if you have time, adding the content in README to the doc?
Thank you.

@showuon showuon merged commit 918a67e into apache:trunk Nov 26, 2024
3 checks passed
@fvaleri fvaleri deleted the docker-readme branch November 26, 2024 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker Official Docker image docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants