initial modules support #3
Conversation
| @@ -0,0 +1,3 @@ | |||
| module github.com/scylladb/go-set | |||
|
|
|||
| require github.com/fatih/set v0.2.1 | |||
There was a problem hiding this comment.
I do not like it. It's only needed for the benchmark.
There was a problem hiding this comment.
This is what happened when I followed the instructions in https://github.com/golang/go/wiki/Modules I don't really like it either...
There was a problem hiding this comment.
We can probably just remove it but I am not sure how it would work for a user running go test -bench ./... for example.
There was a problem hiding this comment.
I'm not really following vgo advancements maybe it's smart enough to figure it out (dep can do that).
There was a problem hiding this comment.
Go modules' go.mod does not distinguish between test dependencies and non-test dependencies. Hence, because github.com/fatih/set is a dependency of one or more of github.com/scylladb/go-set/... packages or their (external) tests, then it is added.
@mmatczuk do you have a concern about this dependency being included?
@dahankzter - any reason you're choosing to ignore go.sum?
There was a problem hiding this comment.
That's good, thanks @myitcv I will add it when the time comes for it. Is the file format frozen yet?
There was a problem hiding this comment.
Module support in Go 1.11 is still considered experimental, so never say never, but I would highly surprised if any changes were made that were backwards incompatible.
There was a problem hiding this comment.
Just to support and expand slightly on that comment from @myitcv: the core go team has said the current file format understood by vgo and go 1.11 will be understood by future tooling... for example in this sentence at the end of the official Modules proposal document:
Even though the details may change, future releases will be able to consume Go 1.11-compatible source trees. For example, Go 1.12 will understand how to consume the Go 1.11 go.mod file syntax, even if by then the file syntax or even the file name has changed.
There was a problem hiding this comment.
I have rerun the procedure using the latest beta (3) and committed the go.sum file as well.
There was a problem hiding this comment.
One other quick comment is that to make it clearer to your users at a glance as to why this dependency is there, you could add a manual comment in the go.mod file. Something like: // only imported when running benchmarks or however you would want to characterize it to help make it clear that it is not used outside of testing or benchmarks.
dahankzter
force-pushed
the
add_modules_support
branch
from
5595d19
to
79a67d9
Compare
dahankzter
force-pushed
the
add_modules_support
branch
from
79a67d9
to
eda4221
Compare
I am really not sure if this is how to do it... I think for us to release v1.0.0 we would need to also create a v1.0.0 tag.