Improving getblock method with more verbosity.
This is really useful when the requester need the whole block.
Details
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- rpcExtend
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 2771 Build 3652: Bitcoin ABC Buildbot (legacy) Build 3651: arc lint + arc unit
Event Timeline
src/rpc/blockchain.cpp | ||
---|---|---|
758 | Need to add some notes in doc/release-notes.md to indicate that getblock verbose param has been deprecated in favor of verbosity. | |
771 | Conceptually this change is ok, but changing verbose -> verbosity is backwards incompatible. It's probably worth adding verbose back, marking it as (deprecated), and if it has an explicit value set, use verbose's value but only if verbosity isn't set (verbosity should take precedence). |
Also, please change the diff title to something like Improving getblock by replacing verbose with verbosity to make it more descriptive.
src/rpc/blockchain.cpp | ||
---|---|---|
771 ↗ | (On Diff #4168) | I don't think it makes sense to have mixed type inputs. I think what you had before (verbosity for numeric) made more sense. However, keeping the verbose (boolean) param will be necessary for backwards compatibility. See my previous comment for a suggestion on implementation. |
822 ↗ | (On Diff #4168) | Looks like you broke something. The rest of the diff is missing? |
src/rpc/blockchain.cpp | ||
---|---|---|
771 ↗ | (On Diff #4168) | I don't think so. If this not good enough for you, please edit it or post me back if this tool don't allow to edit. For backward compatibility this is perfect. |
822 ↗ | (On Diff #4168) | IDK where is it, but the rest are already changed in this file. I think this tool shows now the diff of the last commit. |
This seems to only change the text of some doc. What's the goal here ?
Also, "tested locally" is not the kind of test plan anyone can reproduce.