Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Apr 2, 2016

Should be trivial to review, since it's move only besides a missing return true;.

@paveljanik
Copy link
Contributor

Concept ACK.

I think that the return true is to have the same prototypes of both Warning and Error functions. Is it worth it?

@maflcko
Copy link
Member Author

maflcko commented Apr 2, 2016

Is it worth it?

No, InitError should fail and trigger a shutdown. InitWarning, however, should not trigger a shutdown and continue execution. Thus the return value is not only unused but also misleading.

@paveljanik
Copy link
Contributor

@MarcoFalke Of course I agree with you :-)

@jonasschnelli
Copy link
Contributor

Concept ACK.
Not sure if the function names should contain the Init* part (like InitError() and InitWarning()). From the ui_interface perspective, the error/warning is unrelated to the init process.

@maflcko
Copy link
Member Author

maflcko commented Apr 3, 2016

the error/warning is unrelated to the init process.

This is a valid point, but I think there is no actual use case for a modal pop up other than in the init process. I'd rather keep the old name, and change it when there is a valid use case for a blocking dialog.

@laanwj
Copy link
Member

laanwj commented Apr 3, 2016

Concept ACK

@maflcko
Copy link
Member Author

maflcko commented Apr 18, 2016

Anything holding this back?

@laanwj
Copy link
Member

laanwj commented Apr 19, 2016

Nit: the UI interface should be in bitcoin_server.a, not bitcoin_util.a. It should not be used by the other tools, just bitcoind and bitcoin-qt.

It is only needed by bitcoind and bitcoin-qt
@maflcko
Copy link
Member Author

maflcko commented Apr 19, 2016

Addressed nit

@laanwj laanwj merged commit fa10ce6 into bitcoin:master Apr 19, 2016
laanwj added a commit that referenced this pull request Apr 19, 2016
fa10ce6 Move ui_interface.cpp to libbitcoin_server_a_SOURCES (MarcoFalke)
fabbf80 [ui] Move InitError, InitWarning, AmountErrMsg (MarcoFalke)
@maflcko maflcko deleted the Mf1604-move-ui-helpers branch April 19, 2016 14:45
codablock pushed a commit to codablock/dash that referenced this pull request Dec 20, 2017
fa10ce6 Move ui_interface.cpp to libbitcoin_server_a_SOURCES (MarcoFalke)
fabbf80 [ui] Move InitError, InitWarning, AmountErrMsg (MarcoFalke)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants