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

remove pinning of versions #84

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

drewwells
Copy link
Contributor

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.


.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) .
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

@kd7lxl kd7lxl Mar 3, 2021

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

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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.
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.

3 participants