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

Add go.mod and go.modverify #607

Merged
merged 1 commit into from Dec 28, 2018
Merged

Add go.mod and go.modverify #607

merged 1 commit into from Dec 28, 2018

Conversation

leighmcculloch
Copy link
Contributor

@leighmcculloch leighmcculloch commented May 30, 2018

What

Add go.mod and go.sum, using Go 1.11.

Why

Now that golang/go#24301 has been accepted, lets start using go.mod files
alongside Godep, and keep the two in sync.

Notes

There are no changes required to testify to support go.mod. The files were
generated by running go build and `go mod tidy.

Merging

This PR is intended to be merged after #659 which adds Go1.11 to the list of
supported builds.

@anacrolix
Copy link
Contributor

Works for me.

Copy link
Member

@ernesto-jimenez ernesto-jimenez left a comment

Choose a reason for hiding this comment

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

go.mod versions do not match Gopkg.lock

I tried to do this myself, but vgo is panicking for me when trying to add github.com/stretchr/objx.

@leighmcculloch
Copy link
Contributor Author

I can update the files to be correct, but it would be ideal if the v0.1 version tagged on stretchr/objx was corrected to be v0.1.0. I can fix it without that, but it'll be a better experience for all if that can be corrected. See stretchr/objx#80.

@leighmcculloch
Copy link
Contributor Author

Actually, I don't think that will be enough to make this work. We're still dependent on golang/go#25604 / golang/go#24103. It'd still be good to fix the version on objx though.

@ernesto-jimenez ernesto-jimenez mentioned this pull request Jun 14, 2018
@ernesto-jimenez
Copy link
Member

If we can pull some data about the usage of objx with testify that proofs it is not used, I would consider a breaking change to dump the dependency.

That feature predates my involvement in the project and I have never used it myself and don’t know anybody who has.

@leighmcculloch
Copy link
Contributor Author

Using vgo latest (sha: 0f3e556) that contained fix golang/go#24103, it now works out of the box without any hacks. I've updated this PR as such.

@leighmcculloch
Copy link
Contributor Author

Note: The reason it defines the version for objx as v0.0.0-20180106011353-facf9a85c22f is because the Gopkg.lock file specifies that commit, which is tag v0.1, but that tag is of an invalid version format, it needs to be in v0.1.0, which I raised here stretchr/objx#80 (comment).

@hanzei
Copy link
Contributor

hanzei commented Jun 26, 2018

Can we use require github.com/stretchr/objx v0.1.1?

@leighmcculloch
Copy link
Contributor Author

leighmcculloch commented Jun 27, 2018

👍 I'll update the PR.

@leighmcculloch
Copy link
Contributor Author

I've fixed the vgo files so they have proper versions now. stretchr/objx#80 fixed the v0.1 version so it'd work with vgo by tagging it as v0.1.0.

@leighmcculloch
Copy link
Contributor Author

leighmcculloch commented Jun 29, 2018

The tip build is 🔴but I don't think it's related to this change since go isn't using either of the files being added unless you're using vgo. The other builds are 👍.

@leighmcculloch leighmcculloch mentioned this pull request Jul 16, 2018
@leighmcculloch
Copy link
Contributor Author

leighmcculloch commented Sep 9, 2018

@ernesto-jimenez The go.mod now matches the Gopkg.lock per above comments. Also, I recreated the files against Go1.11's go mod command and the files are identical, so I think this is good to go. What do you think?

It would be ideal if #659 is merged first, but this PR also includes updating .travis.yml so that CI will run tests with Go1.11 both with modules turned on and off.

@leighmcculloch
Copy link
Contributor Author

This branch is now mergeable with master now that #659 is merged. I double checked and it still aligns with the Gopkg.lock on master.

@hanzei
Copy link
Contributor

hanzei commented Nov 13, 2018

What is the status on this PR?

@leighmcculloch
Copy link
Contributor Author

leighmcculloch commented Nov 13, 2018

I'd like to get it merged, it's good to go, I just need someone to review it and merge it.

@dolmen
Copy link
Collaborator

dolmen commented Nov 16, 2018

This PR is quite dirty. Many commits, including reverts and too many of them are merge commits.

Please cleanup: rebase on top of latest master and push again:

git remote add origin https://github.com/stretchr/testify
git remote update
git rebase vgo upstream/master
git push -f

@leighmcculloch
Copy link
Contributor Author

@dolmen I squashed the changes into a single commit so that it looks more organized for you. Ready to go!

@leighmcculloch
Copy link
Contributor Author

The build failure looks like it might be related to a change on the tip version of Go and looks unrelated to the changes in this PR.

dolmen
dolmen previously approved these changes Nov 16, 2018
Copy link
Collaborator

@dolmen dolmen left a comment

Choose a reason for hiding this comment

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

👍 (but I'm not a maintainer)

@leighmcculloch
Copy link
Contributor Author

@ernesto-jimenez Now that #659 has been merged, are we good to merge this?

@leighmcculloch
Copy link
Contributor Author

@ernesto-jimenez 👋 Is this something we can merge now, or can you tell me if there other things I can address? Or are you wanting to hold off on adopting Go modules?

What
===
Add `go.mod` and `go.sum`, using `Go 1.11`.

Why
===
Now that golang/go#24301 has been accepted, lets start using go.mod files
alongside Godep, and keep the two in sync.

Notes
===
There are no changes required to testify to support go.mod. The files were
generated by running `go build` and `go mod tidy.

Merging
===
This PR is intended to be merged after #659 which adds Go1.11 to the list of
supported builds.
@georgelesica-wf
Copy link

Taking a look, I'll get it merged ASAP.

@georgelesica-wf georgelesica-wf added this to the Next Release milestone Dec 28, 2018
@georgelesica-wf georgelesica-wf merged commit c45a138 into stretchr:master Dec 28, 2018
@georgelesica-wf
Copy link

Thank you for your contribution @leighmcculloch !

@leighmcculloch
Copy link
Contributor Author

No worries! Thanks for getting this merged @georgelesica-wf and @benechols-wf !!!

@leighmcculloch
Copy link
Contributor Author

@georgelesica-wf @benechols-wf What would you think about dropping the existing Gopkg.* files now that we have the go.mod file? Now that we have both it may be difficult to keep them in sync the next time the dependencies need to change.

@georgelesica-wf
Copy link

I would prefer to leave them both for now. The deps don't change all that often, and vgo is still very new, so many people won't have it installed, particularly if they typically work with an older version of Go (which we still support).

@SamWhited
Copy link

SamWhited commented Jan 2, 2019

FWIW, all currently supported versions of Go and one previous version (Go 1.9.7+, 1.10.3+, and of course Go 1.11) all have at least basic read support for Modules. Anything older than 1.10 isn't getting security patches anymore so it may not be worth going out of your way to maintain support for it.

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.

None yet

8 participants