sergi / go-diff

Diff, match and patch text in Go
MIT License
1.81k stars 207 forks source link

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

Closed creachadair closed 4 years ago

creachadair commented 5 years ago

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 commented 5 years ago

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

creachadair commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

creachadair commented 5 years ago

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 commented 5 years ago

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

creachadair commented 5 years ago

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

creachadair commented 5 years ago

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

creachadair commented 5 years ago

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 commented 5 years ago

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

creachadair commented 5 years ago

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

paper777 commented 5 years ago

@sergi PING PTAL Thanks!

creachadair commented 5 years ago

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

creachadair commented 5 years ago

@sergi Please take a look when you are able.

creachadair commented 5 years ago

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

creachadair commented 5 years ago

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

creachadair commented 5 years ago

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

edocevol commented 5 years ago

@sergi Please review PR~~~

creachadair commented 5 years ago

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!

creachadair commented 5 years ago

Thank you!

creachadair commented 5 years ago

Would you like me to squash the commits before merge?

creachadair commented 5 years ago

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

creachadair commented 5 years ago

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

creachadair commented 4 years ago

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.