refactoring: Cleanup StartRest() #13938

1. 
   DesWurstes commented at 3:05 PM on August 10, 2018: contributor

   No description provided.

2. fanquake added the label Refactoring on Aug 10, 2018
3. 
   domob1812 commented at 5:50 PM on August 10, 2018: contributor

   utACK 13938/commits/923b9f5172d5a8d2841fb52e187bd057570853ee

4. 
   MarcoFalke commented at 6:13 PM on August 10, 2018: member

   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?

5. MarcoFalke requested review from practicalswift on Aug 10, 2018
6. 
   MarcoFalke commented at 6:13 PM on August 10, 2018: member

   utACK 923b9f5172d5a8d2841fb52e187bd057570853ee

7. 
   DesWurstes commented at 6:40 PM on August 10, 2018: contributor

   Yes, I saw this one while reviewing that Pull Request. I don’t know if there are more of those.

8. 
   Empact commented at 9:13 PM on August 10, 2018: member

   utACK 923b9f5

9. in src/init.cpp:739 in 923b9f5172 outdated

    735 | @@ -736,8 +736,8 @@ static bool AppInitServers()
    736 |      StartRPC();
    737 |      if (!StartHTTPRPC())
    738 |          return false;
    739 | -    if (gArgs.GetBoolArg("-rest", DEFAULT_REST_ENABLE) && !StartREST())
    740 | -        return false;
    741 | +    if (gArgs.GetBoolArg("-rest", DEFAULT_REST_ENABLE))
   

   ---

   ---

   

   sipa commented at 10:35 PM on August 10, 2018:

   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.

   ---

   

   DesWurstes commented at 6:35 AM on August 11, 2018:

   Done

10. 
    sipa commented at 10:36 PM on August 10, 2018: member

    utACK 923b9f5172d5a8d2841fb52e187bd057570853ee

11. Cleanup StartRest() 2da54f5a66
12. 
    Empact commented at 6:53 AM on August 11, 2018: member

    re-utACK 2da54f5

13. 
    practicalswift commented at 8:17 AM on August 11, 2018: contributor

    utACK 2da54f5a66f91306c064d685bffcee2062b6ebd2

14. practicalswift approved
15. 
    MarcoFalke commented at 10:44 AM on August 11, 2018: member

    utACK 2da54f5

16. donaloconnor approved
17. 
    donaloconnor commented at 2:09 PM on August 11, 2018: contributor

    utACK https://github.com/bitcoin/bitcoin/commit/2da54f5a66f91306c064d685bffcee2062b6ebd2

18. 
    fanquake commented at 9:46 AM on August 12, 2018: member

    utACK 2da54f5

19. fanquake renamed this:
    [Refactoring] Trivial: Cleanup StartRest()
    refactoring: Cleanup StartRest()
    on Aug 12, 2018
20. MarcoFalke added this to the milestone 0.18.0 on Aug 12, 2018
21. 
    laanwj commented at 9:32 AM on August 13, 2018: member

    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?

    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.

22. 
    DesWurstes commented at 12:12 PM on August 13, 2018: contributor

    Maybe we should aim to replace exceptions with returning bool (except in tests)?

23. ken2812221 referenced this in commit f083ec13c3 on Aug 13, 2018
24. MarcoFalke merged this on Aug 13, 2018
25. MarcoFalke closed this on Aug 13, 2018

    ---

26. PastaPastaPasta referenced this in commit 4716319203 on Jul 18, 2021
27. PastaPastaPasta referenced this in commit 1009132cda on Jul 18, 2021
28. PastaPastaPasta referenced this in commit fda1c17fdb on Jul 19, 2021
29. MarcoFalke locked this on Sep 8, 2021

Contributors

DesWurstesdomob1812MarcoFalkeEmpactsipapracticalswiftdonaloconnorfanquakelaanwj

Milestone
0.18.0