Skip to content
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

fix travis-ci #27

Closed
wants to merge 6 commits into from
Closed

fix travis-ci #27

wants to merge 6 commits into from

Conversation

gernest
Copy link
Contributor

@gernest gernest commented Apr 10, 2019

No description provided.

gernest added 4 commits April 10, 2019 10:33
Signed-off-by: gernest <[email protected]>

fix vetting in pre-commit check

Signed-off-by: gernest <[email protected]>

fix vetting in pre-commit check

Signed-off-by: gernest <[email protected]>
Signed-off-by: gernest <[email protected]>
@gernest gernest changed the title wip: fix travis-ci fix travis-ci Apr 10, 2019
@@ -1,8 +1,8 @@
language: go
go:
- 1.7
- 1.12
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is 1.12 now the earliest version of go that we can build on? If so, we should update the README, but I'd prefer us to maintain compatibility with slightly older versions if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am preparing for introducing go modules.
this should also work with 1.7 without any issues. Only context package will limit how far we can go with go versions (can't remember when it was introduced but it is not available in old versions)

#24 is the endgame. So I will be providing a series of patches and will update README accordingly.

@gernest
Copy link
Contributor Author

gernest commented Apr 22, 2019

Gentle ping @richvdh , anything else you want me to do for this?

@turt2live turt2live requested a review from richvdh April 23, 2019 01:42
@turt2live
Copy link
Member

(I suspect he's busy dealing with Unexpected Other Things, but have gently asked for review for when he returns)

@@ -1,8 +1,8 @@
language: go
go:
- 1.7
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(sorry for sitting on this).

If we're preserving compatibility with 1.7, it seems best to leave this as 1.7 so that we can check that. If the project builds ok on 1.7 there's a good chance it will also build ok on 1.12. The reverse is not true.

@richvdh
Copy link
Member

richvdh commented Jun 18, 2019

I'm afraid I remain confused about what a bunch of the changes here were for - certainly they weren't necessary to "fix travis-ci" as the PR subject claimed (see #29).

In any case I think the relevant changes have now been pulled into #29, #30 and #31, so I'm going to go ahead and close this. Thanks for contributing.

@richvdh richvdh closed this Jun 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants