-
Notifications
You must be signed in to change notification settings - Fork 77
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
Improve makefile and little cleanups #169
Conversation
- Adds targets to do the Workload API code generation using the proper toolchain versions. - Otherwise reorganizes and cleans up the Makefile - Cleans up some linting failures Signed-off-by: Andrew Harding <aharding@vmware.com>
f5d353f
to
a5028d4
Compare
# Toolchain | ||
############################################################################# | ||
|
||
go_version_full := 1.13.15 |
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.
Do we want/need to document supported go version(s) somewhere else? I think this is the first time it is being codified somewhere?
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.
As a module, the canonical place is the go version in the go.mod?
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.
(since we don't produce an artifact)
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.
Ok fair ... The thing in my mind is that people often ask what the minimum supported go version is for a given lib. We can run the test tooling with a different version, so I don't know that what we put here is strictly the same as what is "supported", but I figured if we are building with 1.13 then maybe that's a good place to start.
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.
Yeah, i think ideally we test with each version between the supported version and the latest. Maybe we can get a CI/CD refresh on this repository with github actions that does that.
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.
Yep ... I went to file an issue for that and found that it already exists #30
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 go directive in go.mod can have some subtle side effects on dependency resolution as well as preventing compilation for using features newer than indicated in the directive.
Thanks @azdagron! |
toolchain versions.