-
Notifications
You must be signed in to change notification settings - Fork 38.6k
[WIP] Refactor: Remove all uses of using namespace in all source files. #9235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
using namespace in all source files.|
concept ACK, the diff is small enough I don't see this is as a change that would ruin history, while still being a large scale [trivial] refactor. |
|
Concept ACK. It has to be done. Do not change |
|
@paveljanik I reverted the univalue changes. Using namespaces anywhere is generally a bad idea. The primary reason is explained better than I can here. The gist of it is captured in this sentence: "Library Foo 2.0 could introduce a function, |
|
FWIW, binary check still passes (updated initial post with output for e81f1a0). |
|
Concept ACK |
|
"Using namespaces anywhere is generally a bad idea." To disambiguate this: The use of namespaces in general is not a bad idea, right? Namespaces can IMO be a useful code-organization principle. But the This changes so many source files though :( Let's make sure that we first merge in the features and fixes that we really want in 0.14, to avoid giving people extra rebasing busywork. |
|
@laanwj Oh, yes apologies about ambiguous wording. If I were to decide there'd be a lot more namespaces in the code, but one step at a time, right? :) There are a lot of files changed, yeah. Luckily, rebasing is extremely trivial and straight-forward, and that goes both ways, so I will gladly rebase this PR once you feel ready for it. A secondary option is that you pick files that you want and I make separate PR:s for those files and we do this iteratively. |
e81f1a0 to
a868216
Compare
|
@kallewoof I've merged this, and can still see lots of usage of |
|
@fanquake I used a "find all" for using namespace and fixed everything, but you're right, somehow there's a lot left. For starters I'm going to fix the leftovers so this PR is complete. Thanks for pointing that out! |
9d76b00 to
a2c6066
Compare
…uding test and bench related files.
a2c6066 to
2bfef03
Compare
|
Concept ACK, but this may be too disruptive for a single PR, perhaps you should consider dividing it in a few smaller ones. Maybe start with namespace std? |
|
Yeah, I started doing that (2 PRs so far; I plan to make the next one whenever the current one is merged to not swamp the members), but felt I could keep this PR around as a central point of sorts. I am closing this one though as it can be closed and still be referenced. |
According to the "Source code organization" section in Developer notes,
using namespaceshould be avoided. This commit removes all uses ofusing namespace <xxx>in all source files, including test and bench related files,as well as the.univaluelibraryThis PR is being split into multiple PR's as described below.
benchandtest(#9281):rpcandscript(#9476):walletandutil*(#9643):Remaining (#9644):