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.

Sign up for GitHub

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

Ensure org name in the import path is always NYTimes #76

Merged
merged 4 commits into from
Feb 11, 2019
Merged

Ensure org name in the import path is always NYTimes #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
81d8200
Add go.mod
Feb 9, 2019
1b2a2e1
readme: make sure docs always use NYTimes
Feb 9, 2019
d19d143
Add a canonical import path
Feb 9, 2019
71f9863
readme: fix license link
Feb 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,5 @@ The docs can be found at [godoc.org][docs], as usual.



[docs]: https://godoc.org/github.com/nytimes/gziphandler
[license]: https://github.com/nytimes/gziphandler/blob/master/LICENSE.md
[docs]: https://godoc.org/github.com/NYTimes/gziphandler
[license]: https://github.com/NYTimes/gziphandler/blob/master/LICENSE
5 changes: 5 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module github.com/NYTimes/gziphandler

go 1.12

require github.com/stretchr/testify v1.3.0
7 changes: 7 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
2 changes: 1 addition & 1 deletion gzip.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package gziphandler
package gziphandler // import "github.com/NYTimes/gziphandler"
Copy link

@marwan-at-work marwan-at-work Feb 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

do we need this vanity import? Pretty sure I think Go Modules ignores it and the old Go will freak out anyway if a user did lower case "get"

Copy link
Contributor Author

@fsouza fsouza Feb 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@marwan-at-work because gziphandler doesn't have sub-packages, there's nothing enforcing what case is used in the import path.

Before this change:

% docker run golang go get -v github.com/nytimes/gziphandler
github.com/nytimes/gziphandler (download)
github.com/nytimes/gziphandler

% docker run golang go get -v github.com/NYTimes/gziphandler
github.com/NYTimes/gziphandler (download)
github.com/NYTimes/gziphandler

Using the CASE-fold org, where the canonical import path is declared:

% docker run golang go get -v github.com/CASE-fold/gziphandler
github.com/CASE-fold/gziphandler (download)
package github.com/CASE-fold/gziphandler: code in directory /go/src/github.com/CASE-fold/gziphandler expects import "github.com/case-fold/gziphandler"

% docker run golang go get -v github.com/case-fold/gziphandler
github.com/case-fold/gziphandler (download)
github.com/case-fold/gziphandler

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(same thing applies to dep)

Copy link

Choose a reason for hiding this comment

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

Oh I see, the lack of sub packages makes the import path work both ways...maybe that's a good thing 😄

Copy link
Contributor Author

@fsouza fsouza Feb 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Not really, because every package in a build have to agree on the import path, so if one of your dependencies wants to use nytimes while others want to use NYTimes, you're busted.

This is what happened to logrus: while it does have sub-packages, the package github.com/sirupsen/logrus doesn't depend on any of the them. The internet crashed once some packages started using sirupsen while others were still using Sirupsen.

Copy link

Choose a reason for hiding this comment

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

very good point ^


import (
"bufio"
Expand Down