Page MenuHomePhabricator

Removed using namespace std and replaced with std:: throughout file
ClosedPublic

Authored by AtlasShrugging on Jul 27 2017, 04:08.

Details

Summary

as described

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

AtlasShrugging created this revision.Jul 27 2017, 04:08
Owners added a reviewer: Restricted Owners Package.Jul 27 2017, 04:08
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 27 2017, 04:08
AtlasShrugging edited the summary of this revision. (Show Details)Jul 27 2017, 04:32
CCulianu added a subscriber: CCulianu.EditedJul 27 2017, 05:54

I am all in favor of doing this. I never import the std namespace into my projects. I am pretty sure most C++ programmers frown on that and it's another amanteur hour thing to do so in my mind, so a huge thumbs up on this from me.

I defer to more senior project members such as deadalnix and freetrader on this one to actually approve this change -- but you got my vote, if that counts.

freetrader added a subscriber: freetrader.EditedJul 27 2017, 07:55

@AtlasShrugging re: your Test Plan question:

As a bare minimum when you touch the code, you should specify that the unit tests are run (make check)
If the change is functional, you may want to add running of the regression tests (qa/pull-tester/rpc-tests.py)
Have a look at the commit history for more examples.

When a change requires a test that cannot be automated, then specify the test steps, conditions, input data and expected outputs as clearly as possible so the next person has a good chance to reproduce the test without having to ring you up :-)

p.s. You can edit the summary and test plan of the Diff as you need.
In this case 'make check' is fine for tests and let the summary just describe the changes you made, briefly.
If the Diff title is enough to express that, just use 'as described' for the summary.

freetrader requested changes to this revision.EditedJul 27 2017, 09:30

@AtlasShrugging : I reviewed and ran the tests over your code, it's good, but please amend the Summary and Test Plan based on our discussion above, then I can accept...

This revision now requires changes to proceed.Jul 27 2017, 09:30
AtlasShrugging edited the summary of this revision. (Show Details)Jul 27 2017, 14:43
AtlasShrugging edited the test plan for this revision. (Show Details)
AtlasShrugging edited the summary of this revision. (Show Details)
AtlasShrugging requested review of this revision.Jul 27 2017, 14:47
AtlasShrugging edited edge metadata.
freetrader accepted this revision.Jul 27 2017, 14:48

All good

This revision is now accepted and ready to land.Jul 27 2017, 14:48
This revision was automatically updated to reflect the committed changes.