Page MenuHomePhabricator

Make Config more available in Qt
ClosedPublic

Authored by dagurval on Nov 23 2017, 14:11.

Details

Diff Detail

Repository
rABC Bitcoin ABC
Branch
23-11-configaround
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 1159
Build 1159: arc lint + arc unit

Event Timeline

dagurval created this revision.Nov 23 2017, 14:11
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 23 2017, 14:11

I like the idea, please just make sure you use a pointer when you take ownership of the Config object down the road.

src/qt/bitcoin.cpp
219

pointer

367

Pointer

src/qt/bitcoingui.h
55

because you take ownership, you shouldn't use a reference. Using reference usually imply you won't take ownership of things (it's not a hard rule, more like a common convention).

134

Pointer

deadalnix requested changes to this revision.Nov 23 2017, 15:00
This revision now requires changes to proceed.Nov 23 2017, 15:00
dagurval added inline comments.Nov 23 2017, 15:07
src/qt/bitcoingui.h
55

I'm not taking ownership

dagurval requested review of this revision.Nov 27 2017, 08:06
deadalnix added inline comments.Nov 29 2017, 14:49
src/qt/bitcoingui.h
55

You are keeping a reference to it.

deadalnix requested changes to this revision.Dec 4 2017, 18:26

Please use a pointer when you keep a reference to something.

This revision now requires changes to proceed.Dec 4 2017, 18:26
dagurval updated this revision to Diff 1944.Dec 5 2017, 22:31

use pointers

schancel accepted this revision.Dec 6 2017, 20:29
deadalnix accepted this revision.Dec 6 2017, 21:23
This revision is now accepted and ready to land.Dec 6 2017, 21:23
This revision was automatically updated to reflect the committed changes.