Skip to content

Add go.mod and go.sum files to support Go modules. #98

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

Merged
merged 1 commit into from
Nov 19, 2019
Merged

Add go.mod and go.sum files to support Go modules. #98

merged 1 commit into from
Nov 19, 2019

Conversation

creachadair
Copy link
Contributor

This commit contains no functional changes; it's just adding config files for
the Go modules facility. Ideally this commit should also be accompanied by a
new version tag, since the current latest tag is v1.0.0 from Nov. 2017, and
several optimizations have been added since then.

@creachadair
Copy link
Contributor Author

Clearly the CI is in some way discontent with this change, I'm trying to work out why.

@creachadair
Copy link
Contributor Author

creachadair commented Feb 4, 2019

Ah, I think I see the problem. The last successful CI build was 6 months ago on #94, and since then the errcheck golint package has updated to depend on Go ≥ 1.9. The Travis config for 1.8 will therefore no longer work at all, irrespective of modules.

Edit: Correct which package was afflicted.

@creachadair
Copy link
Contributor Author

Indeed, CI fails at HEAD: https://travis-ci.org/sergi/go-diff/jobs/488608466. So this PR didn't cause any new problems, but there are some existing ones that will need fixed. I'll look into that separately.

@creachadair
Copy link
Contributor Author

Ok, I was able to fix the import path issue for golang.org/x/lint for 1.9, but 1.8 remains broken because the API is no longer compatible. Specifically, 1.8 predates type alias support.

@creachadair
Copy link
Contributor Author

I suggest dropping support for Go 1.8 entirely. Even though this repo only depends on golint as a verification tool, the dependency compatibility problem runs deeper. To address it you'll need to either install a newer version of Go to build golint, or find an older binary release to depend on (which golint does not itself provide).

@creachadair
Copy link
Contributor Author

After that somewhat noisy debugging, I think this is ready for a review.

@creachadair
Copy link
Contributor Author

I'm not sure who maintains the package, but if someone would be willing to take a look and let me know if this seems reasonable, I'd be grateful! I'm happy to address any concerns promptly.

@creachadair
Copy link
Contributor Author

Ping! Is someone available to take a look and let me know whether you consider this a reasonable change? Thanks.

@creachadair
Copy link
Contributor Author

Ping! @sergi do you have any thoughts about this proposed change?

@creachadair
Copy link
Contributor Author

Ping! Is anyone maintaining this package, and willing to offer comment about this change?

@creachadair
Copy link
Contributor Author

Ping! Can someone please take a look at this change? Even if you have concerns or objections, it would be helpful for us to know what they are. Thanks.

@creachadair
Copy link
Contributor Author

Ping! Is someone available to take a look and let me know whether you consider this a reasonable change? Thanks.

@creachadair
Copy link
Contributor Author

Ping! Can you please take a look when you have time?

@paper777
Copy link

@sergi PING
PTAL
Thanks!

@creachadair
Copy link
Contributor Author

Ping, @sergi. Please have a look when you can, thanks.

@creachadair
Copy link
Contributor Author

@sergi Please take a look when you are able.

@creachadair
Copy link
Contributor Author

Hi, @sergi, I'd be grateful for a review when you are able. Thanks!

@creachadair
Copy link
Contributor Author

Greetings, @sergi! Please take a look when you have some time.

@creachadair
Copy link
Contributor Author

Dear @sergi, can you please have a look at this PR when you have time? Thanks!

@edocevol
Copy link

edocevol commented Jul 3, 2019

@sergi Please review PR~~~

@creachadair
Copy link
Contributor Author

Hello, @sergi, when you have some available bandwidth, I'd be very grateful for a review here. If you have any questions or concerns about the change, I'll do my best to address them. Or if you disagree with the change entirely, it would be helpful to know why. Thanks!

Copy link
Owner

@sergi sergi left a comment

Choose a reason for hiding this comment

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

Sorry to be so late reviewing this. It should be good to go now. Thanks!

@creachadair
Copy link
Contributor Author

Thank you!

@creachadair
Copy link
Contributor Author

Would you like me to squash the commits before merge?

@creachadair
Copy link
Contributor Author

I went ahead and pre-squashed the commits so the merge will be clean once CI completes.

@creachadair
Copy link
Contributor Author

Hi, @sergi, I think this is ready for a merge now (I don't have write access). Thanks!

@creachadair
Copy link
Contributor Author

Hi, @sergi, I've updated the Travis CI config to include Go 1.13 now that it has been released, and added the new Go tool version tag to go.mod. Once CI clears this should be ready to merge at your convenience.

This commit contains no functional changes; it's just adding config files for
the Go modules facility. Ideally this commit should also be accompanied by a
new version tag, since the current latest tag is v1.0.0 from Nov. 2017, and
several optimizations have been added since then.

Related changes:

- Update to the import canonical path for golint.
- Add Go 1.10, 1.11, 1.12, and 1.13 to CI; drop 1.8.
@creachadair creachadair requested a review from sergi October 18, 2019 19:58
@sergi sergi merged commit 58c5cb1 into sergi:master Nov 19, 2019
@creachadair creachadair deleted the gomod branch November 19, 2019 15:50
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.

4 participants