Use string interpolation for default value of -listen #27232

pull ekzyis wants to merge 1 commits into bitcoin:master from ekzyis:use-string-interpolation-for-default-value-of-listen changing 1 files +1 −1
  1. ekzyis commented at 9:55 AM on March 9, 2023: none

    This is a refactoring change. So I have read the following and will try to answer why this change should be accepted

    • Refactoring changes are only accepted if they are required for a feature or bug fix or otherwise improve developer experience significantly. For example, most "code style" refactoring changes require a thorough explanation why they are useful, what downsides they have and why they significantly improve developer experience or avoid serious programming bugs. Note that code style is often a subjective matter. Unless they are explicitly mentioned to be preferred in the developer notes, stylistic code changes are usually rejected.

    I have noticed in #26899 (review) that the helper message for -listen does not use string interpolation.

    That confused me and I wasn't sure what the reasons for that are. So it could be argued this confusion (by possibly many people in the past and in the future) may already be enough to accept this change.

    However, not accepting this means that if DEFAULT_LISTEN is ever changed, this helper message will still use the old value (however unlikely that may be).

    Therefore, this PR makes the helper message consistent with how other helper messages are implemented (using string interpolation) which leads to less confusion and prevents possibly wrong documentation in the future.

  2. Use string interpolation for default value of -listen 5c938e74cf
  3. DrahtBot commented at 9:55 AM on March 9, 2023: 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
    ACK vasild, stratospher, kristapsk

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #26899 (p2p: set -dnsseed and -listen false if maxconnections=0 by brunoerg)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  4. vasild approved
  5. vasild commented at 12:26 PM on March 9, 2023: contributor

    ACK 5c938e74cfdf6b89c6c4d5cce2fd07cbfc8b29c2

  6. stratospher commented at 1:37 PM on March 9, 2023: contributor

    ACK 5c938e7.

  7. kristapsk approved
  8. kristapsk commented at 4:15 PM on March 9, 2023: contributor

    cr utACK 5c938e74cfdf6b89c6c4d5cce2fd07cbfc8b29c2

  9. fanquake merged this on Mar 10, 2023
  10. fanquake closed this on Mar 10, 2023

  11. sidhujag referenced this in commit 9ac4af26d7 on Mar 10, 2023
  12. bitcoin locked this on Mar 9, 2024
  13. ekzyis deleted the branch on Nov 13, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 21:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me