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

Add Ping() to v2 client. #5598

Merged
merged 2 commits into from
Feb 10, 2016
Merged

Add Ping() to v2 client. #5598

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed650cf
Add Ping() to v2 client.
PSUdaemon Feb 9, 2016
7d44293
Update comments to be more idiomatic.
PSUdaemon Feb 9, 2016
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
Prev Previous commit
Update comments to be more idiomatic.
  • Loading branch information
@PSUdaemon
PSUdaemon committed Feb 9, 2016
commit 7d44293506f3970e84bcdb63fa1b9255e2153f42
9 changes: 5 additions & 4 deletions client/v2/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@ type BatchPointsConfig struct {

// Client is a client interface for writing & querying the database
type Client interface {
// Ping will check to see if the server is up with an optional timeout on waiting for leader.
// Ping returns how long the request took, the version of the server it connected to, and an error if one occurred.
// Ping checks that status of cluster
Ping(timeout time.Duration) (time.Duration, string, error)

// Write takes a BatchPoints object and writes all Points to InfluxDB.
Expand Down Expand Up @@ -123,7 +122,8 @@ func NewHTTPClient(conf HTTPConfig) (Client, error) {
}, nil
}

// Pings cluster
// Ping will check to see if the server is up with an optional timeout on waiting for leader.
// Ping returns how long the request took, the version of the server it connected to, and an error if one occurred.
func (c *client) Ping(timeout time.Duration) (time.Duration, string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

golint won't like this comment. Can you put something more descriptive, beginning with Ping.... Possibly take your description on the interface and put it here, with a short description on the interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't mind making this change and in fact I already have it in my tree, but I went to confirm that golint was happy and it seems it doesn't actually care about the plural form. Are there some options I need to use to get it to be strict about that?

$ golint
client.go:24:6: exported type HTTPConfig should have comment or be unexported
client.go:50:6: exported type UDPConfig should have comment or be unexported
client.go:60:6: exported type BatchPointsConfig should have comment or be unexported
client.go:91:1: comment on exported function NewHTTPClient should be of the form "NewHTTPClient ..."
client.go:321:6: exported type Point should have comment or be unexported
client.go:364:1: comment on exported method Point.Tags should be of the form "Tags ..."

Let me know if I should push my updated comments for consistency. I can also fix the other comments in a separate PR.

Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess my comment was meant more as "put the guts of the description on the method doc, and the shorter description on the interface field" 😄

And yes, we always appreciate some golint tidying!

now := time.Now()
u := c.url
Expand Down Expand Up @@ -196,7 +196,8 @@ func NewUDPClient(conf UDPConfig) (Client, error) {
}, nil
}

// Pings cluster
// Ping will check to see if the server is up with an optional timeout on waiting for leader.
// Ping returns how long the request took, the version of the server it connected to, and an error if one occurred.
func (uc *udpclient) Ping(timeout time.Duration) (time.Duration, string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment needs to begin with Ping...

return 0, "", nil
}
Expand Down