-
Notifications
You must be signed in to change notification settings - Fork 197
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
chore: fixes vanity import. #960
Conversation
7f1cc95
to
4d17406
Compare
💔 Tests Failed
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪Test errorsExpand to view the tests failures
|
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.
thanks! I'd just like to shuffle the Makefile change up to be with the other conditional "check-" targets please
Makefile
Outdated
@@ -85,3 +85,7 @@ model/marshal_fastjson.go: model/model.go | |||
|
|||
scripts/Dockerfile-testing: $(wildcard module/*) | |||
go generate ./scripts | |||
|
|||
.PHONY: check-vanity-import | |||
check-vanity-import: |
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.
can you please move this up with the other conditional targets at line 15? and define an empty target for Go < 1.12
78cc889
to
077516b
Compare
@axw I addressed your comments. |
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.
If we can change go get
to go install
for consistency that'd be great. Otherwise LGTM, thanks!
Tests are failing but pretty certain that's unrelated, so merging. Thanks! |
Oops, looks like it's related to the testify upgrade actually. I'll sort it out. |
* chore: fixes vanity import. * chore: uses v0.1.0 for porto. * chore: use install instead of get for tools.
This PR adds a check for vanity imports to be updated on CI.
Ping @axw
Closes #844