-
Notifications
You must be signed in to change notification settings - Fork 76
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
CLOUDP-126655: golangci-lint isn't properly installed by Makefile #1334
Conversation
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 main reason why requesting changes is that then projects that have different versions will clash with it each other, see terraform which is a couple of version behind
Also tho has gotten better support installing from source is not recommended by the golangci-lint project, there are known issues
Finally the problem here is not an installation issue but a PATH export issue which only happens in mac
@gssbzn I've replicated the change you've mentioned |
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.
LGTM
@gssbzn sorry, one more, I've realized we were missing one tool |
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.
LGTM good work with this one, specially the windows checks
.PHONY: setup | ||
setup: deps setupgolangcilint ## Set up dev env | ||
.PHONY: devtools | ||
devtools: ## Install dev 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.
Feel free to do as a follow up
https://github.com/mongodb/mongodb-atlas-cli/blob/master/CONTRIBUTING.md#environment
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.
@gssbzn I'm not too sure we need to change anything since make setup
is still available.
I've only added make devtools
as separated target because evergreen would benefit from it.
Proposed changes
golangci-lint isn't properly installed by Makefile, this PR changes the install location to be
$GOPATH/bin
instead of/path/to/mongodb-atlas-cli/bin
and adds it to$PATH
when running commands.Jira ticket: CLOUDP-126655
Closes #[issue number]
Checklist
make fmt
and formatted my codeFurther comments
This PR changes:
go env GOPATH
) which makes it globally availablemake setup
(github.com/google/go-licenses
)