RPC: Internal named params #17356
RPC: Internal named params #17356
Conversation
|
Well, my hope is that after this is merged, we can migrate existing code to use it and drop |
|
The |
|
100 args are only needed to throw an internal bug report. You can replace that with a check that checks if the first argument is the string "throw_internal_bug_report" (or anything else you can come up 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. |
8186d29 to
e52fa66
Compare
|
Concept ACK |
|
Alternative fix for |
…cts instead of entire JSONRPCRequests
With internal named parameters, all parameters MUST have names
2735696 to
f47bd98
Compare
|
Rebased. Thanks to changes in master, neither hack is needed anymore. |
|
🐙 This pull request conflicts with the target branch and needs rebase. Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft". |
|
Doesn't seem to be much point in rebasing this. It'd be nice to have, but without any review interest, just wasting time. |
|
Concept ACK |
| static inline JSONRPCRequest transformArguments(const JSONRPCRequest& in, const std::vector<std::string>& argNames) | ||
| { | ||
| auto out = in; | ||
| if (in.params.isObject()) { |
There was a problem hiding this comment.
Nit: wouldn't it be slightly cleaner to do something like:
bool isPositional = in.params.isObject();
out.m_used_positional_args = isPositional;
if (isPositional) { ... }
Same loc count, but reads slightly cleaner IMHO.
| out.params = transformNamedArguments(in.params, argNames); | ||
| } else { | ||
| out.m_used_positional_args = true; | ||
| if (in.params.size() > argNames.size()) { |
There was a problem hiding this comment.
Is there some reason to do this after setting out.m_used_positional_args? Otherwise it seems either cleaner to put it first, or to put it last and take out the return out;.
Reworked #11660 to add a new
m_paramstoJSONRPCRequestin parallel to the legacyparamsto ease the transition.Dropped other unnecessary improvements and "options" handling for now to keep this PR smaller, those can go in later in other PRs.