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

Support Go Modules #249

Merged
merged 1 commit into from Dec 20, 2018

Conversation

Projects
None yet
2 participants
@SamWhited
Copy link
Contributor

SamWhited commented Dec 18, 2018

Hello,

Please consider supporting Go Modules, the new packaging standard that will be adopted fully in Go 1.12. Experimental support is in Go 1.11 and the new module paths are supported in Go 1.9.7+ and Go 1.10.3+ in a read-only manner for backwards compatibility with all supported versions of Go.

Because this library is still below version 2 and is already using semver compatible tags, the go.mod file is fairly simple.

It should also be noted that I picked 1.5 as the version of the Go language being used because it was the earliest version that the CI config was testing against; this won't break compatibility with 1.5, but it won't be able to use the mod file and I'm not sure if the new tooling actually supports warning about using newer language features and APIs that far back. At worse though this is no different than today and nothing will be broken.

Thank you for your consideration.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Dec 18, 2018

Codecov Report

Merging #249 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #249      +/-   ##
=========================================
+ Coverage   19.69%   19.7%   +0.01%     
=========================================
  Files          17      17              
  Lines        1442    1441       -1     
=========================================
  Hits          284     284              
+ Misses       1141    1140       -1     
  Partials       17      17
Impacted Files Coverage Δ
tscreen_linux.go 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af444b9...7cf8e26. Read the comment docs.

@gdamore

This comment has been minimized.

Copy link
Owner

gdamore commented Dec 18, 2018

Waiting for CI/CD. I may want to rethink our use of goconvey in the future. .. it brings in a lot of additional dependencies, which most of the times we don't need (only for test).

Probably also I will want to bump the go version -- go versions below 1.9 won't see that file anyway, and I need to stop trying to support go versions that the go team have deprecated.

@SamWhited SamWhited force-pushed the SamWhited:support_modules branch from f6a53cf to 7ebd2b3 Dec 19, 2018

@SamWhited

This comment has been minimized.

Copy link
Contributor Author

SamWhited commented Dec 19, 2018

Probably also I will want to bump the go version -- go versions below 1.9 won't see that file anyway, and I need to stop trying to support go versions that the go team have deprecated.

Sounds good; I pushed a change to test/support Go 1.9+ instead.

EDIT:

I may want to rethink our use of goconvey in the future. .. it brings in a lot of additional dependencies, which most of the times we don't need (only for test).

It appears to only be my machine, but on Go 1.12 beta1 it actually causes a panic when running tests for me. I have no idea why; something deep in the bowels of goconvey. Go 1.11 works fine and tests pass. I'm not sure why that would be, but it does seem like a lot of additional complexity just to run tests.

EDIT2: Oh goodness, I just noticed it's using gls (and that appears to be where the problem is) no wonder it's failing.

@SamWhited SamWhited referenced this pull request Dec 19, 2018

Merged

Support Go Modules #7

@SamWhited SamWhited force-pushed the SamWhited:support_modules branch from 7ebd2b3 to a70eaa7 Dec 19, 2018

@gdamore

This comment has been minimized.

Copy link
Owner

gdamore commented Dec 20, 2018

I believe I've removed the dependency on goconvey. Please do a go mod tidy to see if that clears it up.

@SamWhited SamWhited force-pushed the SamWhited:support_modules branch from a70eaa7 to 7cf8e26 Dec 20, 2018

@SamWhited

This comment has been minimized.

Copy link
Contributor Author

SamWhited commented Dec 20, 2018

LGTM; thanks!

@gdamore gdamore merged commit aaadc57 into gdamore:master Dec 20, 2018

4 of 5 checks passed

Codacy/PR Quality Review Hang in there, Codacy is reviewing your Pull request.
Details
codecov/patch Coverage not affected when comparing af444b9...7cf8e26
Details
codecov/project 19.7% (+0.01%) compared to af444b9
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@SamWhited SamWhited deleted the SamWhited:support_modules branch Dec 20, 2018

@SamWhited

This comment has been minimized.

Copy link
Contributor Author

SamWhited commented Dec 20, 2018

Thanks for the merge! Any chance this can be tagged with a version I can pin too now? Thanks again.

@gdamore

This comment has been minimized.

Copy link
Owner

gdamore commented Dec 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment