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

Converting to go 1.11 modules #210

Merged
merged 1 commit into from
Dec 2, 2018
Merged

Converting to go 1.11 modules #210

merged 1 commit into from
Dec 2, 2018

Conversation

smyrman
Copy link
Collaborator

@smyrman smyrman commented Nov 30, 2018

Removes vendored version of hysterix.

Initattes two new go module roots:

  • One for the git root
  • One for the examples/ sub-directory

Resolves #209

@smyrman smyrman changed the title Converting to go 1.11 modules WIP converting to go 1.11 modules Nov 30, 2018
@smyrman smyrman force-pushed the smyrman/gomod branch 6 times, most recently from 480b286 to d65f9da Compare November 30, 2018 23:53
@smyrman
Copy link
Collaborator Author

smyrman commented Dec 1, 2018

The example_test.go was moved to the examples/ folder to get the mod file to detect less dependencies. The examples/ folder got it's own mod file, with a replace directive for the rest-layer dependency.

Any opinions on this approach @rs?

@rs
Copy link
Owner

rs commented Dec 1, 2018

I don't think you need to do that. Just do no run go test before committing the go.mod and test deps won't be added.

@Dragomir-Ivanov
Copy link
Contributor

go mod init github.com/rs/rest-layer; go mod tidy

Always includes test deps, without any go test run, after the above command.

@smyrman
Copy link
Collaborator Author

smyrman commented Dec 1, 2018

It's certainly possible to exclude test deps from go.mod, but it doesn't look to be best-practice. From https://github.com/golang/go/wiki/Modules#how-to-prepare-for-a-release, there are two relevant recommendations for preparing a release of your go module:

  1. Run go mod tidy. This command is the only command to remove unused dependencies, but it will also add any test dependencies.
  2. Run go test all [redefined in Go module mode] to test your module (including running the tests for your direct and indirect dependencies) as a way of validating that the currently selected packages versions are compatible.

For the latter, test dependencies are defenantly best added to the go.mod. As @Dragomir-Ivanov pointed out earlier though, test dependencies are not downloaded when building a module that depend on rest-layer; only their go.mod files are. They would be for go test all, of course.

As for moving the exmaple_test.go to examples, a better alternative for keeping the amount of dependencies down in the main module, would be to rewrite it to only depend on the standard library (e.g. http.Handler, the default logger etc).

@smyrman
Copy link
Collaborator Author

smyrman commented Dec 1, 2018

As for adding a separate go module root in the examples/ directory, that makes (as far as I know) anything depending on rest-layer, or commands run against rest-layer, including go test ./..., completely ignore the examples/ directory. I think this is reasonable - the dependencies found in the example are completely irrelevant for rest-layer as a library - but I can't claim there is a precedence in the community for doing it in this way yet.

Removes vendored version of hysterix.

Initattes two new go module roots:
- One for the git root
- One for the examples/ sub-directory

Change example_test.go to rely on fewer external dependenceis.

Updates Travis Go version to 1.11, and enables go build cache for faster
CI runs.
@smyrman
Copy link
Collaborator Author

smyrman commented Dec 2, 2018

As for moving the example_test.go to examples/, a better alternative for keeping the amount of dependencies down in the main module, would be to rewrite it to only depend on the standard library (e.g. http.Handler, the default logger etc).

I have done an attempt on this. Kept the cors dependency as it's hard to avoid, and is already included in another test.

@Dragomir-Ivanov
Copy link
Contributor

cors is fine I guess. It is needed during development, anyway.

@smyrman smyrman changed the title WIP converting to go 1.11 modules Converting to go 1.11 modules Dec 2, 2018
@smyrman
Copy link
Collaborator Author

smyrman commented Dec 2, 2018

I will go ahead and merge this.

@smyrman smyrman merged commit a757914 into master Dec 2, 2018
@smyrman smyrman deleted the smyrman/gomod branch December 2, 2018 22:45
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.

3 participants