cli: Treat high detail levels as maximum in -netinfo #21192
Conversation
I somehow often type `-netinfo 5` which gets treated as `-netinfo 0`, after this change it's `-netinfo 4` which seems more convenient behavior.
There was a problem hiding this comment.
Turning the guitar amp up to 11, I see 🎸
ACK 882ce25
Note that 174ed32 proposes to raise on values > 4 following on our irc convo, which might be more predictable in line with what the help says, but perhaps also more annoying...with this change (and currently) it raises on values > 255. I agree that both changes are better than the current behavior.
Yess.
Ok, that's fine to me too, but if we have the unspoken contract that larger values only add more verbosity I guess I slightly prefer this (so you could use 255 as "give me as much information as supported"). |
|
sgtm 🏎️ 💨 255 |
|
Concept ACK I've always assumed that number goes up implies verbosity goes up. In line with: |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
There was a problem hiding this comment.
Tested ACK 882ce25
Agree that this is closer to what the user expects. This reminds me of old projects (e.g. an early version of nmap) where sometimes developers would call gcc with absurd optimization levels like -O9 ;-) (https://news.ycombinator.com/item?id=18582195)
Turning the guitar amp up to 11, I see 🎸
The spinal taproot? 🤘
…info 882ce25 cli: Treat high detail levels as the maximum in -netinfo (Wladimir J. van der Laan) Pull request description: I somehow often type `-netinfo 5` which gets treated as `-netinfo 0`, after this change it's `-netinfo 4` which seems more convenient behavior. ACKs for top commit: jonatack: ACK 882ce25 theStack: Tested ACK 882ce25 Tree-SHA512: 5ed13213d00940c81f70c5fe5092f5fcc78c0184e1cc83b6c58a7bf24cf0f634816ce28f468aac588a4d202a6a7c1b411c0690099f1a8bf1999e662de4afcccd
7d3343f cli: update -netinfo help doc following the merge of 882ce25 (Jon Atack) ef614bb cli: small -netinfo simplification and performance improvement (Jon Atack) 6b45ef3 cli: improve -netinfo invalid argument error message (Jon Atack) 3732404 cli: warn in help that -netinfo is not intended to be a stable API (Jon Atack) 7afdd72 cli: enable -netinfo help to run without a remote server (Jon Atack) Pull request description: A few updates, some per IRC discussion today at http://www.erisian.com.au/bitcoin-core-dev/log-2021-01-07.html#l-87 with respect to -netinfo: - enable `-netinfo help` to run without a remote server - warn in `-netinfo help` that -netinfo is not intended to be a stable API - improve the -netinfo invalid argument error message - make a performance improvement and simplification I noticed after the merge of #20764 - update the -netinfo help doc following the merge of #21192 ----- How to test manually: 🔬 🧪 📈 1. check out and build this branch locally; if you need help, don't hesitate to refer to https://jonatack.github.io/articles/how-to-review-pull-requests-in-bitcoin-core#pull-down-the-code-locally or https://jonatack.github.io/articles/how-to-compile-bitcoin-core-and-run-the-tests 2. while it is compiling, look at the code changes 3. stop signet (if it is running) with `./src/bitcoin-cli -signet stop` 4. once the build is completed, run `./src/bitcoin-cli -signet -netinfo help` 5. the help should be printed even though the signet server is not running 6. near the top you should see the new warning, "This human-readable interface will change regularly and is not intended to be a stable API" as well as a bit more description about the integer argument values. 7. start signet with `./src/bitcoind -signet` 8. test the improved invalid argument error message if you run `./src/bitcoin-cli -signet -netinfo 256` or `./src/bitcoin-cli -signet -netinfo a` (valid values are from 0 to 255) 9. leave review feedback or `ACK <commit hash>` -- done 🍻 ACKs for top commit: michaelfolkson: Re-ACK 7d3343f pinheadmz: RE-ACK 7d3343f Tree-SHA512: 28c5e9f295ffccba5c2a70faac4987d45f35d4758cf8f10daa767e83212316c4cfc65930e4066f7ad627e9d15b92d43439d1ba9c2f755dfde61885c6a70aa155
I somehow often type `-netinfo 5` which gets treated as `-netinfo 0`, after this change it's `-netinfo 4` which seems more convenient behavior. Github-Pull: bitcoin#21192 Rebased-From: 882ce25 C++11 inexplicably doesn't like the `static constexpr` Moved it where a conflict is likely if it needs updating
I somehow often type
-netinfo 5which gets treated as-netinfo 0, after this change it's-netinfo 4which seems more convenient behavior.