search

purpleidea / mgmt

Next generation distributed, event-driven, parallel config management!
https://purpleidea.com/tags/mgmtconfig/
GNU General Public License v3.0
3.67k stars 315 forks source link

Drop support for Go 1.9 in `make build` #618

Closed frebib closed 3 years ago

frebib commented 3 years ago

Drop support for Go 1.9 in make build

docs/development.md says the minimum required Golang version is 1.13 at the time of writing.

frebib commented 3 years ago

At this point should 1.14 be the "supported" version? I'm not sure what kind of compatibility you want to support. Upstream supports 3 minor releases, which at present is 1.13-1.15

purpleidea commented 3 years ago

One question, otherwise LGTM. (Not entirely sure why you want go install but I don't mind having it there if it's useful for you. (Note we use go build -i which is similar.) Actually can you rename the target to goinstall so we can use the install target for installing it on our system properly eventually?

purpleidea commented 3 years ago

PS: We'll move to 1.14 shortly.

frebib commented 3 years ago

Yeah I can rename the target. The main justification for this is because there are several build steps outside of the go toolchain that are required before go build. It would be much easier if the Go build used go generate for all external build commands, but this works well enough as an alternative.

purpleidea commented 3 years ago

np, just ping when patch is ready

frebib commented 3 years ago

ping @purpleidea Addressed the comments

purpleidea commented 3 years ago

Sorry, I just left two review comments. Not sure if I didn't see those points earlier or if they're new. As for the .PHONY -- if it still works as a new header, I'm fine with that approach! Anything you can do better than I did previously, is of course reasonable to propose! Don't necessarily follow my bad habits when it makes sense to branch out!

purpleidea commented 3 years ago

LGTM

frebib commented 3 years ago

Dropped the go install commit, I don't think it's necessary.

purpleidea commented 3 years ago

Rebased to master and merged, thanks!