Skip to content
This repository has been archived by the owner on Jun 17, 2024. It is now read-only.

Gomod Updates #4

Merged
merged 20 commits into from
Jan 23, 2020
8 changes: 4 additions & 4 deletions boilerplate/lyft/golang_dockerfile/Dockerfile.GoTemplate
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@

# Using go1.10.4
FROM golang:1.10.4-alpine3.8 as builder
RUN apk add git openssh-client make curl dep
RUN apk add git openssh-client make curl

# COPY only the dep files for efficient caching
COPY Gopkg.* /go/src/github.com/lyft/{{REPOSITORY}}/
# COPY only the go mod files for efficient caching
COPY go.mod go.sum /go/src/github.com/lyft/{{REPOSITORY}}/
WORKDIR /go/src/github.com/lyft/{{REPOSITORY}}

# Pull dependencies
RUN dep ensure -vendor-only
RUN go mod download

# COPY the rest of the source code
COPY . /go/src/github.com/lyft/{{REPOSITORY}}/
Expand Down
19 changes: 12 additions & 7 deletions boilerplate/lyft/golang_test_targets/Makefile
Original file line number Diff line number Diff line change
@@ -1,25 +1,30 @@
# WARNING: THIS FILE IS MANAGED IN THE 'BOILERPLATE' REPO AND COPIED TO OTHER REPOSITORIES.
# ONLY EDIT THIS FILE FROM WITHIN THE 'LYFT/BOILERPLATE' REPOSITORY:
#
#
# TO OPT OUT OF UPDATES, SEE https://github.com/lyft/boilerplate/blob/master/Readme.rst

DEP_SHA=1f7c19e5f52f49ffb9f956f64c010be14683468b

Copy link
Member

@honnix honnix Jan 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops it seems I introduced this empty line.

.PHONY: lint
lint: #lints the package for common code smells
which golangci-lint || curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | bash -s -- -b $$GOPATH/bin v1.16.0
golangci-lint run --exclude deprecated
which golangci-lint || make download_tooling
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling make from make feels a bit weird. Is there any reason we have to do this? Can we just make lint depend on download_tooling and push down which check to https://github.com/lyft/boilerplate/pull/4/files#diff-3ec8ae048736f7fca33bd837e03342cbR31

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lint: download_tooling

GL_DEBUG=linters_output,env golangci-lint run --deadline=5m --exclude deprecated -v

# If code is failing goimports linter, this will fix.
# skips 'vendor'
.PHONY: goimports
goimports:
@boilerplate/lyft/golang_test_targets/goimports

.PHONY: install download_tooling mod_download
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

install is declared as phony at https://github.com/lyft/boilerplate/pull/4/files#diff-aff368f43e9bd90b088a8b4e6c01bf8fR25 already. Maybe remove that line? Is the convention here to have phony defined separately for each goal?

download_tooling: #download dependencies (including test deps) for the package
boilerplate/lyft/golang_test_targets/download_tooling.sh

mod_download: #download dependencies (including test deps) for the package
go mod download

.PHONY: install
install: #download dependencies (including test deps) for the package
which dep || (curl "https://raw.githubusercontent.com/golang/dep/${DEP_SHA}/install.sh" | sh)
dep ensure
install:
download_tooling mod_download
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be right after :.

install: download_tooling mod_download


.PHONY: test_unit
test_unit:
Expand Down
4 changes: 2 additions & 2 deletions boilerplate/lyft/golang_test_targets/Readme.rst
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Golang Test Targets
~~~~~~~~~~~~~~~~~~~

Provides an ``install`` make target that uses ``dep`` install golang dependencies.
Provides an ``install`` make target that uses ``go mod`` to install golang dependencies.

Provides a ``lint`` make target that uses golangci to lint your code.

Expand All @@ -17,7 +17,7 @@ Provides a ``test_benchmark`` target for benchmark tests.

Add ``lyft/golang_test_targets`` to your ``boilerplate/update.cfg`` file.

Make sure you're using ``dep`` for dependency management.
Make sure you're using ``go mod`` for dependency management.

Provide a ``.golangci`` configuration (the lint target requires it).

Expand Down
47 changes: 47 additions & 0 deletions boilerplate/lyft/golang_test_targets/download_tooling.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#!/bin/bash

# Everything in this file needs to be installed outside of current module
# The reason we cannot turn off module entirely and install is that we need the replace statement in go.mod
# because we are installing a mockery fork. Turning it off would result installing the original not the fork.
# However, because the installation of these tools themselves sometimes modifies the go.mod/go.sum files. We don't
# want this either. So instead, we're going to copy those files into a temporary directory, do the installation, and=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo and=

# ignore any changes made to the go mod files.
# (See https://github.com/golang/go/issues/30515 for some background context)

go_get_mockery () {
tmp_dir=$(mktemp -d -t gotooling-XXXXXXXXXX)
echo tmp_dir
cp go.mod go.sum "$tmp_dir"
pushd "$tmp_dir"
go get github.com/vektra/mockery/cmd/mockery
popd
}

go_get_pflags () {
tmp_dir=$(mktemp -d -t gotooling-XXXXXXXXXX)
cp go.mod go.sum "$tmp_dir"
pushd "$tmp_dir"
go get github.com/lyft/flytestdlib/cli/pflags
popd
}

go_get_lint () {
tmp_dir=$(mktemp -d -t gotooling-XXXXXXXXXX)
cp go.mod go.sum "$tmp_dir"
pushd "$tmp_dir"
go get github.com/golangci/golangci-lint/cmd/golangci-lint
popd
}

go_get_enumer () {
tmp_dir=$(mktemp -d -t gotooling-XXXXXXXXXX)
cp go.mod go.sum "$tmp_dir"
pushd "$tmp_dir"
go get github.com/alvaroloes/enumer
popd
}

go_get_mockery
go_get_pflags
go_get_lint
go_get_enumer
2 changes: 1 addition & 1 deletion boilerplate/update.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ set -e
DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )"

OUT="$(mktemp -d)"
git clone [email protected]:lyft/boilerplate.git "${OUT}"
git clone --single-branch --branch gomod [email protected]:lyft/boilerplate.git "${OUT}"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will revert this before merging, just here for testing.


echo "Updating the update.sh script."
cp "${OUT}/boilerplate/update.sh" "${DIR}/update.sh"
Expand Down