util: Move error message formatting of NonFatalCheckError to cpp #25112

1. 
   maflcko commented at 7:19 am on May 12, 2022: member

   This allows to strip down the header file.
2. maflcko added the label Refactoring on May 12, 2022
3. maflcko force-pushed on May 12, 2022
4. maflcko force-pushed on May 12, 2022
5. 
   DrahtBot commented at 1:39 pm on May 24, 2022: contributor

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

   Reviews

   See the guideline for information on the review process.

   Type	Count	Reviewers
   ACK	2	aureleoules, hebasto
   Concept ACK	1	laanwj

   Conflicts

   No conflicts as of last run.

6. in src/util/check.h:8 in fa13e2f365 outdated

   4@@ -5,30 +5,23 @@
   5 #ifndef BITCOIN_UTIL_CHECK_H
   6 #define BITCOIN_UTIL_CHECK_H
   7 
   8-#if defined(HAVE_CONFIG_H)
   

   ---

   ---

   

   laanwj commented at 12:33 pm on May 30, 2022:

   Any idea what this bitcoin-config.h was needed for before?

   ---

   

   maflcko commented at 12:39 pm on May 30, 2022:

   This was added without explanation in commit 691c817b340 by @ryanofsky

   ---

   

   ryanofsky commented at 8:49 pm on May 30, 2022:

   This was added without explanation in commit 691c817 by @ryanofsky

   I added the include there to define the PACKAGE_BUGREPORT macro referenced below (it was probably needed previously, but not obvious because of an earlier include)

   ---

   

   maflcko commented at 7:12 am on May 31, 2022:

   Thx.

   In this pull, I moved this to the cpp file, as the PACKAGE_BUGREPORT is only used there. iwyu is also happy on the changes here: https://cirrus-ci.com/task/4985805499269120?logs=ci#L7190

7. 
   laanwj commented at 12:33 pm on May 30, 2022: member

   Concept ACK
8. maflcko force-pushed on May 31, 2022
9. DrahtBot added the label Needs rebase on Jul 7, 2022
10. maflcko force-pushed on Jul 7, 2022
11. DrahtBot removed the label Needs rebase on Jul 7, 2022
12. DrahtBot added the label Needs rebase on Jul 12, 2022
13. maflcko force-pushed on Jul 12, 2022
14. DrahtBot removed the label Needs rebase on Jul 12, 2022
15. maflcko force-pushed on Sep 12, 2022
16. DrahtBot added the label Needs rebase on Nov 3, 2022
17. maflcko force-pushed on Nov 14, 2022
18. maflcko force-pushed on Nov 14, 2022
19. DrahtBot removed the label Needs rebase on Nov 14, 2022
20. 
    hebasto commented at 9:33 am on November 16, 2022: member

    Concept ACK.
21. in src/util/check.cpp:10 in fadbc71e01 outdated

     6+#if defined(HAVE_CONFIG_H)
     7+#include <config/bitcoin-config.h>
     8+#endif
     9 
    10 #include <tinyformat.h>
    11+#include <util/check.h>
    

    ---

    ---

    

    hebasto commented at 11:06 am on November 16, 2022:

    nit: Usually we put the header, which is directly related to the source file, at the top of the headers.

    ---

    

    maflcko commented at 11:24 am on November 16, 2022:

    yes, done
22. in src/util/check.h:24 in fadbc71e01 outdated

    31 T&& inline_check_non_fatal(LIFETIMEBOUND T&& val, const char* file, int line, const char* func, const char* assertion)
    32 {
    33     if (!(val)) {
    34-        throw NonFatalCheckError(
    35-            format_internal_error(assertion, file, line, func, PACKAGE_BUGREPORT));
    36+        throw NonFatalCheckError{assertion, file, line, func};
    

    ---

    ---

    

    hebasto commented at 11:07 am on November 16, 2022:

    nit: The #24812 (review) could be addressed as well here.

    ---

    

    maflcko commented at 11:23 am on November 16, 2022:

    thx, done
23. hebasto approved
24. 
    hebasto commented at 11:07 am on November 16, 2022: member

    ACK fadbc71e010b32e23f158bd95cdf2f26c870e11f, I have reviewed the code and it looks OK, I agree it can be merged.
25. util: Move error message formatting of NonFatalCheckError to cpp

    This allows to strip down the header file

    2222ec71fd
26. maflcko force-pushed on Nov 16, 2022
27. hebasto approved
28. 
    hebasto commented at 11:58 am on November 16, 2022: member

    re-ACK 2222ec71fdf573a15bb593fc0dd42d2d28ca5449, only rebased and suggested changes since my recent review.
29. aureleoules approved
30. 
    aureleoules commented at 12:19 pm on November 16, 2022: member

    ACK 2222ec71fdf573a15bb593fc0dd42d2d28ca5449
31. maflcko merged this on Nov 16, 2022
32. maflcko closed this on Nov 16, 2022

    ---

33. maflcko deleted the branch on Nov 16, 2022
34. sidhujag referenced this in commit 6031a314c2 on Nov 16, 2022
35. bitcoin locked this on Mar 6, 2024

Contributors
maflcko DrahtBot laanwj ryanofsky hebasto aureleoules

Labels
Refactoring