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 1250
Build 1250: 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 ↗(On Diff #1816)

pointer

367 ↗(On Diff #1816)

Pointer

src/qt/bitcoingui.h
55 ↗(On Diff #1816)

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 ↗(On Diff #1816)

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 ↗(On Diff #1816)

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 ↗(On Diff #1816)

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.