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

Update imports to make this repo no longer a fork in terms of import paths #2

Merged
merged 1 commit into from
Aug 23, 2019
Merged

Update imports to make this repo no longer a fork in terms of import paths #2

merged 1 commit into from
Aug 23, 2019

Conversation

leighmcculloch
Copy link

@leighmcculloch leighmcculloch commented Aug 23, 2019

Summary

Update imports to make this repo no longer a fork in terms of import paths. This repo will no longer be github.com/throttled/throttled available at a different URL, instead it will be github.com/bartekn/throttled.

Goal and scope

This package is imported into the stellar/go repository as a fork of github.com/throttled/throttled. In recent versions of Go Modules there appears to be a bug (golang/go#33795) that cause packages that don't have a go.mod when replaced to not be importable. To make migrating to Go Modules (stellar/go#1634) easier it will be lower effort and less blocking to simply change this repository to standalone rather than be a fork. In principle since this fork has diverged significantly it's unlikely to be merged back upstream anyway.

Summary of changes

  • Rename the package path to github.com/bartekn/throttled in doc.go.
  • Rename all self imports from test packages to github.com/bartekn/throttled in _test.go files.

@bartekn bartekn merged commit 3ec6187 into bartekn:master Aug 23, 2019
leighmcculloch added a commit to stellar/go that referenced this pull request Aug 26, 2019
…1642)

Make @bartekn's fork of the `throttled` dependency the primary dependency for that dependency instead of being just a custom source, which is now hosted at `github.com/stellar/throttled`.

This is part of the initiative to move to Modules (#1634) and is a small change to simplify our Gopkg.toml/lock files to make that transition possible and simpler.

Using the fork seems to be confusing the go tooling in go1.12 and go1.13rc1, with both behaving a little differently. There is an issue open golang/go#33795 about it. The issue seems to be limited to replacing modules that don't have a `go.mod`, which is the case with the `throttled` module. Since this fork is the only fork of a dependency we use and it is simple package, it is straight forward for us to switch to using the fork fully in name rather than to try and make it work consistently with both versions, and we did that in bartekn/throttled#2. See that issue for more details about why we felt like this is the right move.

For context, in modules a `replace` directive is how we tell the go toolchain that we want to use a fork of a module. However, a replace statement only applies when go commands are executed inside this repo as the main module and so importers of our repo won't be using the fork. This doesn't really matter for `throttled` because we only use it in `internal` and `main` modules of `horizon`, but because of this the go toolchain does care about the original source since importers of our repo will get the original source, not the replacement.

The revision of `stellar/throttled` that we are importing did change. You can see the diff here:
stellar/throttled@c99eef3...89d7581
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.

2 participants