Util: Remove ArgsManager wrappers: #10119
Conversation
src/util.cpp
Outdated
There was a problem hiding this comment.
Any reason to not return a const ref?
Also, ArgsAt is a bit of an odd name, perhaps ArgsFor or something (but maybe not worth the effort to redo this)
There was a problem hiding this comment.
oh -- I didn't understand 'includes' to mean builds on the PR. gotcha
src/util.cpp
Outdated
There was a problem hiding this comment.
Should this push_back or replace?
There was a problem hiding this comment.
I believe push_back, but it's a good point, I am not sure now.
src/util.cpp
Outdated
There was a problem hiding this comment.
Is the PR too big if you get rid of these?
There was a problem hiding this comment.
The less disruptive to remove are removed in #10119 I'm happy to squash that in #9494
Others are more disruptive to remove, but I'm working on doing it for SoftSetBoolArg and IsArgSet in jtimon/bitcoin@0.14-args-wrappers...jtimon:0.14-args-isargset-wrapper but I'm not sure everybody will like that way of doing it.
src/util.h
Outdated
There was a problem hiding this comment.
I was just using the same that was used before. We can move to multimap, what would be the gain?
There was a problem hiding this comment.
No real gain, just slightly simpler (because multi_map is intended to be used I think exactly in this type of case).
There was a problem hiding this comment.
I thought it was more for when you wanted the elements on the vector to be ordered in certain way or something. Leaving the same structure seems easier to review, but once encapsulated it is easy to make further improvements to ArgsManager's internal implementation.
jtimon
force-pushed
the
0.14-args-wrappers
branch
from
11b82c4 to
1a52af8
Compare
|
Needed rebase. |
jtimon
force-pushed
the
0.14-args-wrappers
branch
from
1a52af8 to
c165600
Compare
- ParseParameters - ReadConfigFile - ForceSetArg - SoftSetArg
jtimon
force-pushed
the
0.14-args-wrappers
branch
from
c165600 to
9d6c531
Compare
|
Incorportated in #9494 |
3 participants
Includes:
From the wrappers newly introduced with ArgsManager, these seem currently the easiest to remove.