refactor: Deleted unreachable code in httpserver.cpp #26570

1. 
   JoaoAJMatos commented at 9:18 PM on November 24, 2022: contributor

   Some of the code in httpserver.cpp was unreachable, and didn't follow the developer notes.

2. Deleted unreachable code in httpserver.cpp 50439383a2
3. 
   DrahtBot commented at 9:18 PM on November 24, 2022: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Concept ACK	stickies-v
   Approach ACK	w0xlt
   Stale ACK	hebasto

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

4. DrahtBot added the label Refactoring on Nov 24, 2022
5. 
   stickies-v commented at 3:17 PM on November 25, 2022: contributor

   Concept ACK - would clean up HTTPRequest::GetRequestMethod() at the same time though. Can't find any other such instances in the codebase (grepped with "return.*\;\n\s*break\;")

6. 
   w0xlt commented at 4:16 PM on November 25, 2022: contributor

   Approach ACK

7. hebasto approved
8. 
   hebasto commented at 4:24 PM on November 25, 2022: member

   ACK 50439383a24fb7096e100ee6383da76db99426d5.

   Agree with the suggestion above.

   Also, while touching this switch statement, maybe it makes sense to follow our Developer Notes and replace default: with case HTTPRequest::UNKNOWN:?

9. Deleted unreachable code & refactored switch statement to follow developer notes c29c4d36a8
10. JoaoAJMatos requested review from w0xlt on Nov 26, 2022
11. JoaoAJMatos removed review request from w0xlt on Nov 26, 2022
12. JoaoAJMatos requested review from stickies-v on Nov 26, 2022
13. JoaoAJMatos removed review request from stickies-v on Nov 26, 2022
14. JoaoAJMatos requested review from w0xlt on Nov 26, 2022
15. Refactored c29c4d36a to prevent compiler warnings in RequestMethodString

    Added an assertion to prevent warning  from firing

    0d8f375913
16. 
    JoaoAJMatos commented at 12:46 AM on November 26, 2022: contributor

    I think this might be it

17. JoaoAJMatos removed review request from w0xlt on Nov 26, 2022
18. JoaoAJMatos requested review from stickies-v on Nov 26, 2022
19. Merge branch 'bitcoin:master' into master 9d32336afb
20. JoaoAJMatos removed review request from stickies-v on Nov 27, 2022
21. JoaoAJMatos requested review from w0xlt on Nov 27, 2022
22. 
    hebasto commented at 2:54 PM on November 27, 2022: member

    @JoaoAJMatos

    Thank you for your contribution!

    Usually, pull requests are required to be merge commit free.

    Also the content of your commits could be more focused and self-contained. For example, the first commit removes all break statements from both RequestMethodString and GetRequestMethod functions. The second one refactors the RequestMethodString function to follow the Developer Notes.

    And please include // no default case, so the compiler can warn about missing cases comment as it is being suggested by the Developer Notes.

23. 
    fanquake commented at 4:46 PM on December 8, 2022: member

    @JoaoAJMatos are you planning on following up here? Please also write a pull request description.

24. Added comment in RequestMethodString

    Added a comment in the switch statement inside RequestMethodString function as suggested by the developer notes

    00bf29e948
25. 
    fanquake commented at 5:15 PM on December 8, 2022: member

    @JoaoAJMatos you need to fixup your changes according to #26570 (comment). You also cannot have merge commits.

26. JoaoAJMatos closed this on Dec 8, 2022

    ---

27. maflcko referenced this in commit 1ea02791f3 on Dec 10, 2022
28. sidhujag referenced this in commit de862d954e on Dec 10, 2022
29. bitcoin locked this on Dec 8, 2023

Contributors

JoaoAJMatosDrahtBotstickies-vw0xlthebastofanquake

Review Requested
w0xlt