Skip to content
This repository has been archived by the owner on Jan 16, 2025. It is now read-only.

all: simpler import path #14

Merged
merged 2 commits into from
Jul 17, 2015
Merged

Conversation

sbinet
Copy link
Member

@sbinet sbinet commented Jun 11, 2015

this CL simplifies the import path from:
github.com/golang/snappy/snappy
to:
github.com/golang/snappy

it also adds the github.com/golang/snappy "vanity" import path to make sure we get only one version of this code.

@dmitshur
Copy link

The new import path is shorter and nicer (with less stutter). Given that the current root folder is basically empty (aside form README, LICENSE, etc.) at this time, I don't see why the Go package cannot live in root. The only reason to keep it in a subdirectory is if it's expected there will be additional Go packages being added to this repo in the future, and each one would be in its own subdirectory.

Since the import path has changed very recently (from code.google.com/p/snappy-go), there may be users who have yet to update their import path, so it wouldn't be any more difficult for them.

For those that have already updated the import path, they'll need to do it once more.

https://godoc.org/github.com/golang/snappy/snappy?importers

There are around 7 existing public packages (plus a few forks) using the new import path.

Is it worth it? I'm not sure. But I think if this change should ever be done, now is the best time.

@nigeltao
Copy link
Contributor

nigeltao commented Jul 6, 2015

I prefer github.com/golang/snappy/snappy. Things like the LICENSE or README file don't belong in the same directory as .go files, and such files have to be at the top level.

@nigeltao nigeltao closed this Jul 6, 2015
@cespare
Copy link

cespare commented Jul 6, 2015

What's wrong with having those files in the same directory as .go files?

Organizing this way makes using the package a little bit less pleasant for all users. Contrast with other github.com/golang packages which are importable at the top level:

  • github.com/golang/glog
  • github.com/golang/groupcache
  • github.com/golang/lint

Not having unnecessary nesting is also the standard practice in the wider community.

@nigeltao
Copy link
Contributor

OK, let's go with the shorter "github.com/golang/snappy" path.

@nigeltao nigeltao reopened this Jul 17, 2015
nigeltao added a commit that referenced this pull request Jul 17, 2015
@nigeltao nigeltao merged commit 7a15765 into golang:master Jul 17, 2015
@sbinet sbinet deleted the simpler-import-path branch July 17, 2015 07:17
@danielchatfield
Copy link

I'm not really sure if anything can be done but I think some discussion should be had on how to make a breaking change like this.

@nigeltao
Copy link
Contributor

Apologies, but I also don't think there's anything obvious I could have done differently. I'm happy to hear any suggestions, though.

@dmitshur
Copy link

I also don't think there's anything obvious I could have done differently. I'm happy to hear any suggestions, though.

I'm glad you merged this PR, as I think the new import path is indeed better and it's not hard to update it in dependent projects.

However, I think you could've done one thing differently to make it a little better. Before merging the PR, you could've made note (and shared here) the list of all publicly known to godoc.org Go packages that used to import the old import path. Basically, the contents of this page, but for the previous import path:

https://godoc.org/github.com/golang/snappy?importers

That would've made it easier to notify or make PRs to affected packages to update the import path from old to new.

There's a limitation with godoc.org in that it will not show who imports a given import path, once there's no longer a valid Go package at that import path. So after you merged this PR, going to https://godoc.org/github.com/golang/snappy/snappy?importers results in 404 instead of a listing of Go packages that need updating. Because you didn't give an advance notice before merging, I (or anyone else) couldn't have looked it up because it was already too late. It's not a huge deal as active users will figure this out on their own, but it'd be nice to know who they were.

For example, when I changed the import path of my github_flavored_markdown package in shurcooL/go#19 (comment), I kept the old version for a month, made PRs to all publicly known importers (https://github.com/search?utf8=%E2%9C%93&q=is%3Apr+author%3AshurcooL+Update+import+path+of+github_flavored_markdown+package) before deleting the old import path.

Again, updating an import path is so easy, that making PRs to fix it is not making it much easier, but I thought that was the most I could do to make it easier for users to update (at least it lets them know about it sooner). I don't think going that far is needed, but I'm just sharing suggestions to consider here.

@losinggeneration
Copy link

@shurcooL makes some good suggestions. Doing pull requests may be a bit overkill if there are lots of projects using it. The old import path could have been left with a log. Print in the init function about the deprecated path. This is a passive way to try to notify people of pending changes.

Another option might be to version branch on breaking changes.

I will counter shurcool's comment about being easy to update dependent projects. It's easy if the project is active. Otherwise, a forked version would be required.

Don't get me wrong though, I too think the new import path is better, and I do appreciate taking suggestions for improvements in the future.

sethvargo added a commit to hashicorp/vault that referenced this pull request Aug 6, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants