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 (max)uploadtarget infos to getnettotals RPC help #6999

Merged

Conversation

jonasschnelli
Copy link
Contributor

@jonasschnelli jonasschnelli commented Nov 12, 2015

No description provided.

@sipa
Copy link
Member

sipa commented Nov 12, 2015

utACK

@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Nov 12, 2015

Same sample output to judge the formatting:
bildschirmfoto 2015-11-12 um 14 43 01

@paveljanik
Copy link
Contributor

paveljanik commented Nov 12, 2015

There are two { on uploadtarget line and missing commas after t...

@paveljanik
Copy link
Contributor

paveljanik commented Nov 12, 2015

Boolean entries are described differently in the other help texts.

@jonasschnelli jonasschnelli force-pushed the 2015/11/maxuploadtarget_rpc_help branch from ca4a1e0 to ddc90ef Compare Nov 12, 2015
@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Nov 12, 2015

Thanks for the review.
Fixed the double "{" and the description for the booleans.

@jonasschnelli jonasschnelli changed the title add (max)uploadtarget infos to getnetinfos RPC help add (max)uploadtarget infos to getnettotals RPC help Nov 12, 2015
@jonasschnelli jonasschnelli force-pushed the 2015/11/maxuploadtarget_rpc_help branch from ddc90ef to 29014b3 Compare Nov 12, 2015
@jonasschnelli jonasschnelli force-pushed the 2015/11/maxuploadtarget_rpc_help branch from 29014b3 to f6d9d5e Compare Nov 12, 2015
@paveljanik
Copy link
Contributor

paveljanik commented Nov 12, 2015

The output here looks like this:

{
  "totalbytesrecv": 7718519,
  "totalbytessent": 56670,
  "timemillis": 1447337892394,
  "uploadtarget": {
    "timeframe": 86400,
    "target": 0,
    "target_reached": false,
    "serve_historical_blocks": true,
    "bytes_left_in_cycle": 0,
    "time_left_in_cycle": 0
  }
}

Please note the position of { in uploadtarget - it is on the same line. Can you move it there also in the helptext?

And this was the last nit.

ACK

@instagibbs
Copy link
Member

instagibbs commented Nov 12, 2015

Would it hurt just to add "bytes" to "target"? aka "targetbytes"
Everything else has bytes in the field name.

nit aside, utACK

Meta-comment: if #6984 gets merged I'd want to know that whitelisted peers existed and I was serving historical blocks to them.

@laanwj
Copy link
Member

laanwj commented Nov 13, 2015

Would it hurt just to add "bytes" to "target"? aka "targetbytes"
Everything else has bytes in the field name.

Hah, yes, we could comment many things on consistency in the RPC interface. Some things have _ separators some have words written together, there are even a few camelCases here and there.

Only if it isn't in a release yet there is still scope for changing it.

@jonasschnelli Thanks for documenting it though - as it is.

@gmaxwell
Copy link
Contributor

gmaxwell commented Nov 13, 2015

utACK

@gmaxwell
Copy link
Contributor

gmaxwell commented Nov 17, 2015

ACK.

@gmaxwell gmaxwell merged commit f6d9d5e into bitcoin:master Nov 17, 2015
1 check passed
gmaxwell added a commit that referenced this issue Nov 17, 2015
f6d9d5e add (max)uploadtarget infos to getnettotals RPC help (Jonas Schnelli)
@dcousens
Copy link
Contributor

dcousens commented Nov 17, 2015

ACK

zkbot added a commit to zcash/zcash that referenced this issue Feb 18, 2021
zkbot added a commit to zcash/zcash that referenced this issue Feb 18, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants