-
Notifications
You must be signed in to change notification settings - Fork 257
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
ci: migrate from travis to github action #391
Conversation
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
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
@natefinch - this drops go 1.9 and 1.10 from the tests. I know how strongly you feel about mage's backwards combability - are you also happy with removing those?
Is there any reason we can't support 1.9 and 1.10 as well? |
@natefinch Go modules was introduced in go1.11, that is the main reason why it does not work for them. |
Oh yeah, lol, yeah, that seems fine. |
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
They should still be backwards compatible. The go tools changed behaviors at go 1.12 to be module aware.
The CI for 1.11.x probably passes because it is defaulting to 1.11.4 which is supported by the repos go.mod version directive. For builds in 1.10.x and 1.9.x, they need to be setup to be in the GOPATH for them to work. I see that you set the GOPATH variable for testing, but the code itself needs to checked out to the GOPATH for the to tooling to work, and it must be in the correct layout for the imports to work. I tried debugging a bit more, but am not as familiar with github actions. As far as I can tell the checkout is not following the expected layout for go version 1.10 and 1.9 to work. see: https://github.com/thaney071/mage/actions Sorry, long post to say the builds should be able to still work using 1.10 and 1.9, but the checkout step needs to be configured so the code is checked out to the GOPATH as: @natefinch seems I took to long to post this. |
This isn't getting picked up in the "checks" section.... I always forget if I need to do something special to turn these things on, or is it just that this is an outside contributor, so it doesn't actually run on the PR? |
Yeah, if we still want to make the builds successful with 1.9 or 1.10, I agree that we can configure GOPATH. But soon or later, we will need to form a plan to remove go1.9 and go1.10 support. |
Since github action is not on the base branch yet, it did not get run. After the merge it would run on the PRs. (that is why I posted the action runs on my fork) |
@natefinch In relation to this, have you thought about adding some anonymous telemetry collection when OS version, go version, build duration, which features of mage are used? How many are using mage with go 1.9/1.10, or mage flags ( I've added ^^^ for the discussion here not to pollute this PR. |
If @natefinch and @mirogta are okay with not testing on go 1.9 and 1.10, this can be merged. I can keep looking at the action setting or update the docs/web template for the tested versions. The website has 1.7 as the lowest tested as of today. |
I think testing on 1.11+ should be fine. That's fall 2018 and was when modules were first added as opt-in. |
closes #378