init: settings, do not load auto-generated warning msg #29301

1. 
   furszy commented at 0:05 am on January 24, 2024: member

   Fixes #29144 (comment).

   The settings warning message is meant to be used only to discourage users from modifying the file manually. Therefore, there is no need to keep it in memory.

2. init: settings, do not load auto-generated warning msg

   The settings warning message is meant to be used only to discourage
   users from modifying the file manually. Therefore, there is no need
   to keep it in memory.

   987a1b51ee
3. 
   DrahtBot commented at 0:05 am on January 24, 2024: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	achow101, ryanofsky

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

4. fanquake requested review from achow101 on Jan 24, 2024
5. fanquake requested review from ryanofsky on Jan 24, 2024
6. fanquake requested review from hebasto on Jan 24, 2024
7. 
   achow101 commented at 5:52 pm on January 24, 2024: member

   ACK 987a1b51eeb72c7fcb085470817a4fe85fcc3c7c
8. DrahtBot removed review request from achow101 on Jan 24, 2024
9. in src/common/settings.cpp:35 in 987a1b51ee

   30@@ -31,6 +31,9 @@ enum class Source {
   31    CONFIG_FILE_DEFAULT_SECTION
   32 };
   33 
   34+// Json object key for the auto-generated warning comment
   35+const std::string SETTINGS_WARN_MSG_KEY{"_warning_"};
   

   ---

   ---

   

   ryanofsky commented at 6:28 pm on January 24, 2024:

   In commit “init: settings, do not load auto-generated warning msg” (987a1b51eeb72c7fcb085470817a4fe85fcc3c7c)

   Not sure, but maybe this can be changed to std::string_view to avoid the need to allocate memory before main() runs.

   ---

   

   furszy commented at 10:21 pm on January 24, 2024:

   In commit “init: settings, do not load auto-generated warning msg” (987a1b5)

   Not sure, but maybe this can be changed to std::string_view to avoid the need to allocate memory before main() runs.

   As the settings map take strings for keys, wouldn’t this change mean that we would need to static cast string_view to string to not create a copy during the push/erase methods call?

   ---

   

   ryanofsky commented at 4:34 pm on February 6, 2024:

   re: #29301 (review)

   As the settings map take strings for keys, wouldn’t this change mean that we would need to static cast string_view to string to not create a copy during the push/erase methods call?

   The idea is that just that it is noticeable when programs are slow to start, so you usually want to reduce the amount of code that needs to run before main(), even if it means code that runs later but is less sensitive might be slower. In this case there is not even a need for a tradeoff, though since univalue could take string_view arguments. Just explaining the suggestion though, this is not important

   ---

   

   furszy commented at 5:15 pm on February 7, 2024:

   Sure, sounds good @ryanofsky. Thanks for coming back to this!
10. ryanofsky approved
11. 
    ryanofsky commented at 6:30 pm on January 24, 2024: contributor

    Code review ACK 987a1b51eeb72c7fcb085470817a4fe85fcc3c7c. Seems like a clean, simple fix
12. glozow requested review from fanquake on Jan 29, 2024
13. 
    fanquake commented at 10:31 am on January 31, 2024: member

    Seems fine. Gets rid of the pointless 2024-01-31T10:25:34Z Ignoring unknown rw_settings value _warning_ and 2024-01-31T10:25:34Z Setting file arg: _warning_ = "This file is automatically generated and updated by Bitcoin Core. Please do not edit this file while the node is running, as any changes might be ignored or overwritten." output.
14. fanquake added this to the milestone 27.0 on Jan 31, 2024
15. achow101 merged this on Jan 31, 2024
16. achow101 closed this on Jan 31, 2024

    ---

17. furszy deleted the branch on Jan 31, 2024

Contributors
furszy DrahtBot achow101 ryanofsky fanquake

Review Requested
hebasto

Milestone
27.0