refactoring: Cleanup StartRest() #13938
Conversation
|
utACK 13938/commits/923b9f5172d5a8d2841fb52e187bd057570853ee |
|
If my memory doesn't fail me, we did something similar only 2 weeks ago: Return void instead of bool for functions that cannot fail #13774 How many are there still left? |
|
utACK 923b9f5172d5a8d2841fb52e187bd057570853ee |
|
Yes, I saw this one while reviewing that Pull Request. I don’t know if there are more of those. |
|
utACK 923b9f5 |
sipa
left a comment
sipa
left a comment
There was a problem hiding this comment.
utACK 923b9f5172d5a8d2841fb52e187bd057570853ee
src/init.cpp
Outdated
There was a problem hiding this comment.
Nit: if you're modifying this code anyway, update it to follow the style guide and put braces around then clause which isn't on the same line.
|
re-utACK 2da54f5 |
|
utACK 2da54f5 |
|
utACK 2da54f5 |
|
utACK 2da54f5 |
fanquake
changed the title
I think this is a good point, I'm also a bit divided on changes like this. It seems unnecessary and exposes callee implementation details to the caller. A consistent convention for error handling ("return false on error") makes sense and if a function cannot return an error that might mean that some potential errors go ignored, or do a lazy thing such as assert out. All this needs to be re-added when proper error handling is introduced in the future. |
|
Maybe we should aim to replace exceptions with returning bool (except in tests)? |
2da54f5 Cleanup StartRest() (DesWurstes) Pull request description: Tree-SHA512: 7e907315009c0351b7a3347ec13b6727abd12fe722d51cc061cb635ea20f9a550af5f50dc364c4313501b0dfc3696bcfa26a2a5f0170a4b5808624e043085d29
9 participants
No description provided.