Add make container-image to README instructions #49444
Conversation
Signed-off-by: Dmitry Shurupov <[email protected]>
k8s-ci-robot
added
the
sig/docs
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
cncf-cla: yes
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -52,9 +52,12 @@ To build the site in a container, run the following: | |||
|
|
|||
| ```bash | |||
| # You can set $CONTAINER_ENGINE to the name of any Docker-like container tool | |||
| make container-image | |||
There was a problem hiding this comment.
I wouldn't mention this here.
Instead, add a (shell) comment that directs people to a page within https://kubernetes.io/docs/contribute/docs/
Most people won't need to do a container image build; if we give them this advice, they'll build the image (slow) rather than pulling it (relatively fast).
There was a problem hiding this comment.
I agree with @sftim. Building a container image is slow. Pulling an existing one is faster.
| make container-serve | ||
| ``` | ||
|
|
||
| The first command (`make container-image`) is required only for the first-time build OR if one of `Dockerfile`, `Makefile`, `package.json`, `package-lock.json` was changed. |
There was a problem hiding this comment.
nit: use bold rather than capitals for emphasis
|
/hold I'm confident this is actually the wrong fix. We could still make a different update to the README. |
k8s-ci-robot
added
the
do-not-merge/hold
| make container-serve | ||
| ``` | ||
|
|
||
| The first command (`make container-image`) is required only for the first-time build OR if one of `Dockerfile`, `Makefile`, `package.json`, `package-lock.json` was changed. |
There was a problem hiding this comment.
This is not correct. Line 33 says otherwise.
If this is a recent bug, I'll suggest opening a separate issue first.
What should it be about? Otherwise, after opening #49460, I think we can close this PR. |
Following a recent discussion on Slack, I'm suggesting to add the
make container-imagecommand to README instructions on running the website using a container.