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

make lint doesn't install golinter if found one locally #999

Merged
merged 3 commits into from
Oct 25, 2019

Conversation

zen-dog
Copy link
Contributor

@zen-dog zen-dog commented Oct 25, 2019

Additionally, renamed code-formatting CircleCI job to lint.

so that `make lint` can run offline. Additionally, renamed `code-formatting` CircleCI job to `lint`.
Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

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

LGTM :) Even better solution would be to wrap it in a shell script and do something similar to what we do here https://github.com/dcos/metronome/blob/master/ci/si_install_deps.sh#L19

@zen-dog
Copy link
Contributor Author

zen-dog commented Oct 25, 2019

LGTM :) Even better solution would be to wrap it in a shell script and do something similar to what we do here https://github.com/dcos/metronome/blob/master/ci/si_install_deps.sh#L19

How does this sentence start with "Even better solution" and ends with "shell script"? 😆

@alenkacz
Copy link
Contributor

LGTM :) Even better solution would be to wrap it in a shell script and do something similar to what we do here https://github.com/dcos/metronome/blob/master/ci/si_install_deps.sh#L19

How does this sentence start with "Even better solution" and ends with "shell script"? 😆

You can choose another technology/language 😝

@nfnt
Copy link
Member

nfnt commented Oct 25, 2019

Try this:

.PHONY: lint
lint:
ifeq (, $(shell which golangci-lint))
	go get github.com/golangci/golangci-lint/cmd/golangci-lint
endif
	golangci-lint run

And it would be great if we could pin the version of golangci-lint, e.g., add

GOLANGCILINT_VERSION ?= "v1.21.0"

at the beginning of Makefile and later

	go get github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCILINT_VERSION)

@zen-dog
Copy link
Contributor Author

zen-dog commented Oct 25, 2019

And it would be great if we could pin the version of golangci-lint, e.g., add

As discussed offline, we want to keep it up-to-date and should any discrepancies between local and CI environment occur, we can revisit this.

Copy link
Member

@nfnt nfnt left a comment

Choose a reason for hiding this comment

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

🚢

@zen-dog zen-dog changed the title Created a separate make target to install latest golinter version make lint doesn't install golinter if found one locally Oct 25, 2019
@zen-dog zen-dog merged commit f5d1519 into master Oct 25, 2019
@zen-dog zen-dog deleted the ad/install-linter branch October 25, 2019 12:58
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.

None yet

3 participants