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

README: use apt-get --assume-yes in Dockerfile #415

Merged
merged 2 commits into from
May 2, 2020
Merged

README: use apt-get --assume-yes in Dockerfile #415

merged 2 commits into from
May 2, 2020

Conversation

Minoru
Copy link
Contributor

@Minoru Minoru commented Apr 30, 2020

This little change makes it even easier for someone to create their own Dockerfile: they simply copy an example and start listing the packages they need. Without --assume-yes, apt-get install will try to interactively ask for confirmation, and fail since docker build runs it in a non-interactive environment.

This little change makes it even easier for someone to create their own
Dockerfile: they simply copy an example and start listing the packages
they need. Without `--assume-yes`, `apt-get install` will try to
interactively ask for confirmation, and fail since `docker build` runs
it in a non-interactive environment.
@Minoru Minoru requested review from Dylan-DPC-zz and a team as code owners April 30, 2020 22:34
@reitermarkus
Copy link
Member

I think all other occurrences are using -y.

@Minoru
Copy link
Contributor Author

Minoru commented May 1, 2020

@reitermarkus I don't really know, I've seen all of the three aliases. I personally think docs should use long options so people don't have to look them up. Then the choice is between --yes and --assume-yes. Now that I think about it, I don't know why I'm always using the latter, but it's just how it is.

I'm okay with changing it to anything else if a reviewer asks for it.

@reitermarkus
Copy link
Member

The main point is that we should be consistent. Feel free to change all Dockerfiles and scripts to use the long option as well.

This makes it consistent with the example in the README.
@Minoru
Copy link
Contributor Author

Minoru commented May 1, 2020

Ah, sorry, I somehow misunderstood you and though that you're talking of all the Dockerfiles in existence, not just the ones in this repo. I've changed them all to use --assume-yes as well.

The "real" Dockerfiles in this repo also use --no-install-recommends, but I decided not to add it to README since the user might actually want recommended packages. The goal of this PR is simply to make the example work out of the box.

@Minoru
Copy link
Contributor Author

Minoru commented May 1, 2020

CI error doesn't look related to my changes at all.

@reitermarkus
Copy link
Member

bors try

bors bot added a commit that referenced this pull request May 1, 2020
@bors
Copy link
Contributor

bors bot commented May 1, 2020

try

Build failed:

@reitermarkus
Copy link
Member

bors retry

bors bot added a commit that referenced this pull request May 2, 2020
@bors
Copy link
Contributor

bors bot commented May 2, 2020

try

Build succeeded:

@reitermarkus
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented May 2, 2020

Build succeeded:

  • rust-embedded.cross

@bors bors bot merged commit de91808 into cross-rs:master May 2, 2020
@reitermarkus
Copy link
Member

Thanks, @Minoru!

@Minoru Minoru deleted the bugfix/assume-yes-in-example-dockerfile branch May 2, 2020 13:33
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