Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Jul 30, 2020

Log a warning to debug log if unknown settings are encountered. This should probably only ever happen when the software is upgraded.

Something similar is already done for the command line and config file. See:

@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 30, 2020

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

Conflicts

No conflicts as of last run.

Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK. This adds useful logging, but see suggestions below because in some ways I think the PR currently does too much.

@maflcko maflcko changed the title Interpret and validate rw_settings Warn on unknown rw_settings Aug 28, 2020
Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK fa48405. Looks good and I could see this being helpful for debugging. Thanks for taking suggestions

@maflcko maflcko merged commit 4769942 into bitcoin:master Oct 19, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 19, 2020
fa48405 Warn on unknown rw_settings (MarcoFalke)

Pull request description:

  Log a warning to debug log if unknown settings are encountered. This should probably only ever happen when the software is upgraded.

  Something similar is already done for the command line and config file. See:

  * test: Add test for unknown args bitcoin#16234 (commit fa7dd88)

ACKs for top commit:
  ryanofsky:
    Code review ACK fa48405. Looks good and I could see this being helpful for debugging. Thanks for taking suggestions

Tree-SHA512: cec7d88adf84fa0a842f56b26245157736eb50df433db951e622ea07fd145b899822b24cdab1d8b36c066415ce4f0ef09b493fa8a8d691532822a59c573aafa7
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 25, 2021
Summary:
Log a warning to debug log if unknown settings are encountered. This should probably only ever happen when the software is upgraded.

Something similar is already done for the command line and config file. See [[bitcoin/bitcoin#16234 | core#16234]]

This is a backport of [[bitcoin/bitcoin#19624 | core#19624]]

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10541
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
@maflcko maflcko deleted the 2007-settings branch April 28, 2022 17:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants