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

Add id and timeoffset to getpeerinfo. #335

Merged
merged 1 commit into from
Mar 10, 2015
Merged

Add id and timeoffset to getpeerinfo. #335

merged 1 commit into from
Mar 10, 2015

Conversation

dajohi
Copy link
Member

@dajohi dajohi commented Mar 1, 2015

No description provided.

@@ -137,6 +137,7 @@ type GetNetworkInfoResult struct {

// GetPeerInfoResult models the data returned from the getpeerinfo command.
type GetPeerInfoResult struct {
Id string `json:"addr"`
Copy link
Member

Choose a reason for hiding this comment

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

This won't be used, so it probably doesn't matter, but json:"id".

@davecgh
Copy link
Member

davecgh commented Mar 1, 2015

This is missing help for the new fields as indicated by the build failure.

rpcserverhelp_test.go:44: Failed to generate help for method 'getpeerinfo': getpeerinforesult-timeoffset

Also, I think Id needs to be ID to keep lint happy.

Version uint32 `json:"version"`
SubVer string `json:"subver"`
Inbound bool `json:"inbound"`
StartingHeight int32 `json:"startingheight"`
CurrentHeight int32 `json:"currentheight,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should be losing this field.

@davecgh
Copy link
Member

davecgh commented Mar 10, 2015

Looks good after you bring the CurrentHeight field back and rebase to latest master.

@davecgh
Copy link
Member

davecgh commented Mar 10, 2015

OK

@conformal-deploy conformal-deploy merged commit db8fa6f into btcsuite:master Mar 10, 2015
@dajohi dajohi deleted the more_getpeerinfo branch March 10, 2015 19:07
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.

3 participants