scripted-diff: use RPCArg::Optional::OMITTED over OMITTED_NAMED_ARG #26919
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. 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. |
|
The scripted diff can be: sed -i -e "/Deprecated alias for OMITTED, can be removed/d" src/rpc/util.{h,cpp}
sed -i -e "s/OMITTED_NAMED_ARG/OMITTED/g" $(git grep -l "OMITTED_NAMED_ARG" src/) |
fanquake
force-pushed
the
remove_omitted_named_arg
branch
from
74453d1 to
0adf96b
Compare
Sure. Updated. |
fanquake
force-pushed
the
remove_omitted_named_arg
branch
from
0adf96b to
92e7a51
Compare
kouloumos
left a comment
kouloumos
left a comment
There was a problem hiding this comment.
ACK 92e7a51579712c8108d565a639b206e7555d58e9
I verified that there are no other occurrences of OMITTED_NAMED_ARG. I've also verified that there is no diff at the rendered RPC doc after this change by using the RPCdump-patch from #14726 (review).
Updated RPCdump-patch
diff --git a/src/init.cpp b/src/init.cpp
index 685583bcb..5120d23b5 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1190,6 +1190,30 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
for (const auto& client : node.chain_clients) {
client->registerRpcs();
}
+ JSONRPCRequest jreq;
+ jreq.mode = JSONRPCRequest::GET_HELP;
+ UniValue unused_result;
+ for (const auto& command : tableRPC.mapCommands) {
+ const CRPCCommand *pcmd = command.second.front();
+ try {
+ pcmd->actor(jreq, unused_result, true);
+ } catch (const std::exception& e) {
+ printf("===\n%s\n", e.what());
+ continue;
+ } catch (const UniValue& e) {
+ printf("===\n%s\n", e.write().c_str());
+ continue;
+ } catch (...) {
+ printf("===\n%s\n", pcmd->name.c_str());
+ fflush(stdout);
+ assert(0);
+ }
+ printf("===\n%s\n", pcmd->name.c_str());
+ fflush(stdout);
+ assert(0);
+ }
+ fflush(stdout);
+ abort();
#if ENABLE_ZMQ
RegisterZMQRPCCommands(tableRPC);
#endif
diff --git a/src/rpc/server.h b/src/rpc/server.h
index 01e855605..130819a56 100644
--- a/src/rpc/server.h
+++ b/src/rpc/server.h
@@ -124,7 +124,7 @@ public:
*/
class CRPCTable
{
-private:
+public:
std::map<std::string, std::vector<const CRPCCommand*>> mapCommands;
public:
CRPCTable();
src/rpc/util.h
Outdated
There was a problem hiding this comment.
Reading this comment and even going through the changes of how this came to be, I still was a bit confused about the options here as part of the overall logic. The removal of OMITTED_NAMED_ARG definitely makes it easier but may I suggest a different wording for the comments:
/**
* Optional argument for which the default value is omitted from
* help text for one of two reasons:
* - It's a named argument and has a default value of `null`.
* - Its default value is implicitly clear. That is, elements in an
* array may not exist by default.
* When possible, the default value should be specified.There was a problem hiding this comment.
Taken your suggestions.
-BEGIN VERIFY SCRIPT- sed -i -e "/Deprecated alias for OMITTED, can be removed/d" src/rpc/util.h src/rpc/util.cpp sed -i -e "s/OMITTED_NAMED_ARG/OMITTED/g" $(git grep -l "OMITTED_NAMED_ARG" src/) -END VERIFY SCRIPT-
fanquake
force-pushed
the
remove_omitted_named_arg
branch
from
92e7a51 to
83f70c8
Compare
|
re-ACK 83f70c8 |
5 participants
Remove deprecated
RPCArg::Optional::OMITTED_NAMED_ARGin favour ofOMITTED.