docs: fix the building part in CONTRIBUTING.md #768
Conversation
CONTRIBUTING.md
Outdated
| in your Deployment specs. | ||
|
|
||
| ### Push the container image to a remote repository | ||
|
|
||
| ```console | ||
| $ TAG=DEV REGISTRY=$USER/kong-ingress-controller make docker-push | ||
| ``` | ||
| in your Deployment specs.` |
There was a problem hiding this comment.
Doesn't look like this is set up to allow suggestions, but I think we should leave in some sort of repo push instructions even if they're not a Makefile target. The container build does still make use of those environment variables when constructing the image name and we should provide basic guidance on pushes.
Something along the lines of:
in your Deployment specs.
### Build and push the container image to a remote repository
export REGISTRY=docker.example.com/registry; export TAG=0.10.0-my-custom-build; make container; docker push $REGISTRY/kong-ingress-controller:$TAG
Note also that there's an extra backtick character at the end of "specs.`" in the current diff that probably wasn't intended/should be removed.
There was a problem hiding this comment.
I have deleted the extra characters.
I think that providing non-existent push instructions may confuse users, so before there is no corresponding push instruction in the Makefile, remove the relevant help from the documentation.
There was a problem hiding this comment.
Having consulted with my colleagues, we respectively disagree here. There's no push in the Makefile because it's not something it needs to automate or can reliably automate well, but it is a task we expect most users will need to perform, and omitting it would result in confusion of its own. Please do re-add that section.
If including the fake example seems confusing, please feel free to add an explanatory "replace docker.example.com/registry with your registry URL" note after, that's fine.
There was a problem hiding this comment.
Ok, I modified some commands and added some tips.
As per commit.