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

Appease golint #11

Closed
wants to merge 1 commit into from
Closed

Appease golint #11

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 18, 2014

Appease golint

Previously there were around a few dozen items it complained about; this eliminates all of those issues.

$ GOPATH/bin/golint . | wc -l
       0
$ go test
PASS
ok      apns    0.013s

Interface Changes

  1. FEEDBACK_TIMEOUT_SECONDS is now FeedbackTimeoutSeconds
  2. APPLE_PUSH_RESPONSES is now ApplePushResponses
  3. PUSH_COMMAND_VALUE is now PushCommandValue
  4. MAX_PAYLOAD_SIZE_BYTES is now MaxPayloadSizeBytes
  5. IDENTIFIER_UBOUND is now IdentifierUbound
  6. TIMEOUT_SECONDS is now TimeoutSeconds

TODOs

Previously there were around a few dozen items it complained about; this eliminates all of those issues.

$ GOPATH/bin/golint . | wc -l
       0
$ go test
PASS
ok      apns    0.013s
@ghost ghost self-assigned this Apr 18, 2014
@ghost ghost added 3 - Done labels Apr 18, 2014
@draaglom
Copy link
Collaborator

I've had a read through, seems good to me 👍

ghost pushed a commit that referenced this pull request Apr 22, 2014
Lately a legitimate number of package managers for Go have cropped up, but this lib came into existence when the options were a little more sparse (i.e. "breaking public API change? Fork the repo!")

This supports #11 and keeps the identifiers around but outside of the normal code...for now. I'll probably wind up forking eventually, but we'll see if we can be creative in the meantime.

$ $GOPATH/bin/golint .
legacy.go:10:2: don't use ALL_CAPS in Go names; use CamelCase
legacy.go:11:2: don't use ALL_CAPS in Go names; use CamelCase
legacy.go:12:2: don't use ALL_CAPS in Go names; use CamelCase
legacy.go:13:2: don't use ALL_CAPS in Go names; use CamelCase
legacy.go:14:2: don't use ALL_CAPS in Go names; use CamelCase
legacy.go:15:2: don't use ALL_CAPS in Go names; use CamelCase

$ go test
PASS
ok      apns    0.013s
@ghost
Copy link
Author

ghost commented Apr 22, 2014

@draaglom cool, thanks! I was thinking about applying b88d4de as a way to eliminate the public interface issue (i.e. the possibility that someone is relying on APPLE_PUSH_RESPONSES and so on).

@draaglom
Copy link
Collaborator

@anachronistic the eternal trade-off of legacy compatibility vs simplicity!

Only you can make that call. My natural inclination - given a choice of now or later - would be to break compatibility now, before more people start using the library.

But on the other hand Go's lack of native versioning makes breaking changes all the more painful. Perhaps breaking compatibility can wait until asynchronous sending (#6) is done.

@ghost
Copy link
Author

ghost commented May 1, 2014

Superseded by #13.

@ghost ghost closed this May 1, 2014
@ghost ghost removed their assignment Aug 8, 2016
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants