-
Notifications
You must be signed in to change notification settings - Fork 46
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
remove pinning of versions #84
base: master
Are you sure you want to change the base?
Conversation
drewwells
commented
Mar 2, 2021
|
||
.PHONY test: test-atlas | ||
test-atlas: fmt-atlas | ||
$(GO_RUNNER) go test $(GO_TEST_FLAGS) $(GO_PACKAGES) | ||
|
||
docker-atlas: | ||
@docker build -f $(SERVER_DOCKERFILE) -t $(SERVER_IMAGE):$(IMAGE_VERSION) . | ||
@docker image prune -f --filter label=stage=server-intermediate | ||
docker build --pull -f $(SERVER_DOCKERFILE) -t $(SERVER_IMAGE):$(IMAGE_VERSION) . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@go mod download | ||
go mod tidy | ||
go mod vendor | ||
go mod download |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there's any value to go mod download
after go mod tidy
/vendor
. It will already have downloaded all the modules.
@@ -1,5 +1,5 @@ | |||
# build the server binary | |||
FROM golang:1.15.7 AS builder | |||
FROM golang:latest AS builder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't lead to reproducible builds.
An alternative approach that would keep versions from going stale but also support reproducible builds would be to add a .github/dependabot.yml
to the templates. Edit: implemented in #90
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should always be fixing bugs on the latest versions of dependencies. Dependabot PR would just fail/get ignored and continue the current process of creating apps on old versions of tools.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since go has been maintaining backwards compatibility for a decade, the only errors I expect to get here are with go test linting rules. Those find bad code so the risk here is quite low while the upsides are high.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the problem with removing the version is that it becomes ambiguous as to which version we are using. Tracing it back to a build would be a PITA. Perhaps we can write a build rule that would check what the latest release is and fail to build unless the user updates it. In any case, let's remove this change and deal with it in a separate PR.
Code that templates out applications with no migration path should not pin versions. It invariably leads to code that stagnates on outdated versions of tools. Removed the masking of makefile output. It provides little value and makes it harder to debug what make is doing.
7b0047f
to
0668378
Compare