Enable local selection of dependencies #83
Conversation
| CLIENT_GEN_VERSION ?= v0.26.1 | ||
| CONTROLLER_GEN_VERSION ?= v0.9.2 | ||
| GOLANGCI_LINT_VERSION ?= v1.52.0 | ||
| MOQ_VERSION ?= v0.3.4 |
There was a problem hiding this comment.
As mentioned before, I would drop these from here and they shouldn't really be configurable on a given branch.
There was a problem hiding this comment.
Retracting, given my coment above.
There was a problem hiding this comment.
I would still remove them from here and only specify them in the Dockerfile. What motivation is there for keeping them defined here?
If these are "real" dependencies, then we should manage them using something else so that we can better manage the update lifecycle.
ArangoGutierrez
force-pushed
the
drafix
branch
2 times, most recently
from
cc67a3c to
fcf76c5
Compare
ArangoGutierrez
force-pushed
the
drafix
branch
2 times, most recently
from
7b58c6a to
f7b344f
Compare
Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>
|
|
||
| TARGETS := $(MAKE_TARGETS) $(CMD_TARGETS) | ||
|
|
||
| DOCKER_TARGETS := $(patsubst %,docker-%, $(TARGETS)) | ||
| .PHONY: $(TARGETS) $(DOCKER_TARGETS) | ||
| DOCKERFILE_DEVEL := $(CURDIR)/deployments/container/Dockerfile.devel |
There was a problem hiding this comment.
Could we move this Dockerfile. It is NOT related to deploying this component. It is used to build the go binaries locally.
There was a problem hiding this comment.
I don't agree with the mix that we have here. We now have this Dockerfile defined in two places -- the k8s-test-infra repo and here with no clear dependencies defined between the two. What causes each of these to change? How do we ensure that these are kept in sync.
The original motivation for the k8s-test-infra image was to have a single image to build these components and to be able to reuse that in the CI steps. At the moment, however, we seemt to have moved to native Golang steps -- with the exception of the build step which could also be native in this case.
Could we rather switch to using local images and not depend on k8s-test-infra at all?
There was a problem hiding this comment.
The approach on this Patch was to enable a fall-back option for local development, to enable a developer to try new versions of the same dependencies at will in real-time, without requiring a PR against k8s-test-infra to bump a dependency there. I will file a new patch to propose a better solution that still gives us the flexibility of this patch, but centralizes everything at k8s-test-infra
| CLIENT_GEN_VERSION ?= v0.26.1 | ||
| CONTROLLER_GEN_VERSION ?= v0.9.2 | ||
| GOLANGCI_LINT_VERSION ?= v1.52.0 | ||
| MOQ_VERSION ?= v0.3.4 |
There was a problem hiding this comment.
I would still remove them from here and only specify them in the Dockerfile. What motivation is there for keeping them defined here?
If these are "real" dependencies, then we should manage them using something else so that we can better manage the update lifecycle.
No description provided.