Move src/tinyformat.h to src/tinyformat/tinyformat.h #13846

pull Empact wants to merge 1 commits into bitcoin:master from Empact:tinyformat-subdir changing 13 files +1087 −1057
  1. Empact commented at 5:26 AM on August 2, 2018: member

    For consistency with other bundled dependencies. Having it in the top-level directory makes it clear that it is a library that we're including rather than a file that is a part of the project.

    tinyformat/tinyformat.h and tinyformat/README.md are drawn from the existing sha reference: https://github.com/c42f/tinyformat/tree/689695cf58700e6defe3741829564cd682d5ae57

    Existing modifications to tinyformat are moved to utilstrprintf.h, clearly separating upstream and project code.

    This includes changes in: 695041e4952ea40e0 - util: Update tinyformat 1b8fd35aadfad6a1e - Make tinyformat errors raise an exception instead of assert()ing 9b6d4c5cdc1ad7b12 - Move strprintf define to tinyformat.h 6e5fd003e04b81115 - Move *Version() functions to version.h/cpp 9eaa0afa6ec5d3dd0 - tinyformat: force USE_VARIADIC_TEMPLATES b651270cd6bfdd6d7 - util: Throw tinyformat::format_error on formatting error

    This does not include changes below, which are cosmetic or apparently unintentional: 64fb0ac016c7fd01c - Declare single-argument (non-converting) constructors "explicit" 4d9b4256d89d1f7c6 - Fix typos

    Note I excluded only the tinyformat cpp from the boost lint checks out of an abundance of caution - these files are not actually built into bitcoin so are not at risk of pulling in unwanted dependencies, whereas the header, and other dependencies might be.

  2. Empact commented at 5:28 AM on August 2, 2018: member

    This is an alternative to #13845. This includes just the header and readme, while #13845 includes the full subtree - more automatic checking is possible with that, but at the cost of including support files.

  3. laanwj commented at 8:10 AM on August 2, 2018: member

    Concept ACK

  4. DrahtBot commented at 8:19 AM on August 2, 2018: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->No more conflicts as of last run.

  5. DrahtBot cross-referenced this on Aug 2, 2018 from issue Inline i64tostr and itostr by Empact
  6. DrahtBot cross-referenced this on Aug 2, 2018 from issue Test for Windows encoding issue by ken2812221
  7. DrahtBot cross-referenced this on Aug 2, 2018 from issue PSBT key path cleanups by sipa
  8. DrahtBot cross-referenced this on Aug 2, 2018 from issue Remove the boost/algorithm/string/case_conv.hpp dependency by l2a5b1
  9. DrahtBot cross-referenced this on Aug 2, 2018 from issue [refactor] Fix the chainparamsbase -> util circular dependency by Empact
  10. DrahtBot cross-referenced this on Aug 2, 2018 from issue policy: Report reason inputs are nonstandard from AreInputsStandard by Empact
  11. DrahtBot cross-referenced this on Aug 2, 2018 from issue [bugfix] Fix encoding issue for Windows by ken2812221
  12. DrahtBot cross-referenced this on Aug 2, 2018 from issue util: Make strprintf noexcept. Improve error message on format string error. by practicalswift
  13. laanwj added the label Upstream on Aug 2, 2018
  14. Empact cross-referenced this on Aug 2, 2018 from issue Include tinyformat as a subtree by Empact
  15. Empact force-pushed on Aug 2, 2018
  16. DrahtBot cross-referenced this on Aug 3, 2018 from issue util: Add [[nodiscard]] to all {Decode,Parse}[...](...) functions returning bool by practicalswift
  17. DrahtBot cross-referenced this on Aug 3, 2018 from issue utils: Use _wfopen and _wfreopen on Windows by ken2812221
  18. DrahtBot cross-referenced this on Aug 3, 2018 from issue utils: drop boost::interprocess::file_lock by ken2812221
  19. laanwj commented at 12:46 PM on August 7, 2018: member

    Closing this in favor of #13845, at least I'd say that if we're going to move it anyhow go the full yard and make it a subtree.

  20. laanwj closed this on Aug 7, 2018

  21. laanwj reopened this on Aug 7, 2018

  22. laanwj commented at 12:48 PM on August 7, 2018: member

    Reopening because of @MarcoFalke's concern, agree that #13845 pulls in a bizarrely high number (46!) of files. And the whole intent of tinyformat.h is to have a file that can be easily copied into a project. It's not clear it is worth it.

  23. laanwj added this to the milestone 0.18.0 on Aug 7, 2018
  24. Empact cross-referenced this on Aug 10, 2018 from issue build: Enable -Wdocumentation (clang) if available by practicalswift
  25. DrahtBot cross-referenced this on Aug 11, 2018 from issue refactor: Removal of circular dependency between index/txindex, validation and index/base by mgrychow
  26. Empact force-pushed on Aug 12, 2018
  27. in src/tinyformat/README.md:3 in 8e603876f6 outdated
       0 | @@ -0,0 +1,452 @@
       1 | +# tinyformat.h
       2 | +
       3 | +## A minimal type safe printf() replacement
    


    MarcoFalke commented at 11:58 PM on August 12, 2018:

    No need to add a third party readme. This will show up in all history/logs and greps.

  28. MarcoFalke commented at 11:59 PM on August 12, 2018: member

    Concept ACK

  29. Empact force-pushed on Aug 13, 2018
  30. Empact commented at 2:07 AM on August 13, 2018: member

    Dropped the readme. 👍

  31. in test/lint/README.md:20 in c5ee680fdf outdated
      16 | @@ -17,6 +17,7 @@ To use, make sure that you have fetched the upstream repository branch in which
      17 |  maintained:
      18 |  * for `src/secp256k1`: https://github.com/bitcoin-core/secp256k1.git (branch master)
      19 |  * for `src/leveldb`: https://github.com/bitcoin-core/leveldb.git (branch bitcoin-fork)
      20 | +* for `src/tinyformat`: https://github.com/c42f/tinyformat.git (branch master)
    


    MarcoFalke commented at 11:49 AM on August 27, 2018:

    nit: Not a subtree

  32. MarcoFalke commented at 11:50 AM on August 27, 2018: member

    utACK c5ee680fdfe806ab6ffbfca47f6cd28e4fb4e049

  33. Move src/tinyformat.h to src/tinyformat/tinyformat.h
    For consistency with other bundled dependencies. Having it in the top-level
    directory makes it clear that it is a library that we're including
    rather than a file that is a part of the project.
    
    tinyformat/tinyformat.h and tinyformat/README.md are drawn from the existing
    sha reference:
    https://github.com/c42f/tinyformat/tree/689695cf58700e6defe3741829564cd682d5ae57
    
    Existing modifications to tinyformat are moved to utilstrprintf.h, clearly
    separating upstream and project code.
    
    This includes changes in:
    695041e4952ea40e0 - util: Update tinyformat
    1b8fd35aadfad6a1e - Make tinyformat errors raise an exception instead of assert()ing
    9b6d4c5cdc1ad7b12 - Move strprintf define to tinyformat.h
    6e5fd003e04b81115 - Move `*Version()` functions to version.h/cpp
    9eaa0afa6ec5d3dd0 - tinyformat: force USE_VARIADIC_TEMPLATES
    b651270cd6bfdd6d7 - util: Throw tinyformat::format_error on formatting error
    
    This does not include changes below, which are cosmetic or apparently unintentional:
    64fb0ac016c7fd01c - Declare single-argument (non-converting) constructors "explicit"
    4d9b4256d89d1f7c6 - Fix typos
    
    Note I excluded only the tinyformat cpp from the boost lint checks out of an
    abundance of caution - these files are not actually built into bitcoin so are
    not at risk of pulling in unwanted dependencies, whereas the header, and other
    dependencies might be.
    d48249e240
  34. Empact force-pushed on Aug 28, 2018
  35. Empact commented at 1:13 AM on August 28, 2018: member

    Removed inaccurate test/lint/README mention and unnecessary test/lint/lint-includes.sh filter.

  36. MarcoFalke commented at 7:22 PM on August 29, 2018: member

    re-utACK d48249e2400487fb99a6bb15999eccd489a71318

  37. laanwj commented at 1:37 PM on August 31, 2018: member

    This was discussed at the 2018-08-30 IRC meeting and there was no agreement to do this at all.

    21:20 < wumpus> one topic I'd like to discuss is where to move tinyformat in the source tree, if we're going to do
                    that at all, I hate it when there's two competing PRs open for something
    21:20  * jonasschnelli is lost in PRs
    21:20 < wumpus> #topic tinyformat move
    21:20 < wumpus> e.g.: [#13846](/github-metadata-backup-bitcoin-bitcoin/13846/), [#13845](/github-metadata-backup-bitcoin-bitcoin/13845/), or keep as is
    21:20 < gribble> [#13846](/github-metadata-backup-bitcoin-bitcoin/13846/) | Move src/tinyformat.h to
                     src/tinyformat/tinyformat.h by Empact . Pull Request [#13846](/github-metadata-backup-bitcoin-bitcoin/13846/) . bitcoin/bitcoin . GitHub
    21:20 < gribble> [#13845](/github-metadata-backup-bitcoin-bitcoin/13845/) | Include tinyformat as a subtree by Empact . Pull
                     Request [#13845](/github-metadata-backup-bitcoin-bitcoin/13845/) . bitcoin/bitcoin . GitHubAsset 1Asset 1
    21:21 < wumpus> I'm ok with all three options but not with leaving those PRs open forever
    21:22 < jonasschnelli> The subtree looked to me after an overkill,... I would prefer [#13846](/github-metadata-backup-bitcoin-bitcoin/13846/) (no strong opinion)
    21:22 < gribble> [#13846](/github-metadata-backup-bitcoin-bitcoin/13846/) | Move src/tinyformat.h to
                     src/tinyformat/tinyformat.h by Empact . Pull Request [#13846](/github-metadata-backup-bitcoin-bitcoin/13846/) . bitcoin/bitcoin .
    21:22 < wumpus> I guess MarcoFalke is not here?
    21:23 < wumpus> I think he has the strongest opinion about it
    21:23 < gmaxwell> would we really do a subtree for a single file?
    21:23 < wumpus> no.
    21:24 < wumpus> I think this is pretty much unnecessary, and certainly the subtree one contains lots of changes
    21:24 < gmaxwell> seems like change for the sake of change to me.
    21:24 < wumpus> too much of that
    21:25 < achow101> I'm in favor of keeping it as is
    

    If there's no agreement how to go forward here, I'd say it's not extremely important to do, so I'm closing both PRs for now.

  38. laanwj closed this on Aug 31, 2018

  39. MarcoFalke commented at 2:48 PM on August 31, 2018: member

    Imo this could still be done the next time we touch tinyformat, but yeah no rush

  40. bitcoin locked this on Sep 8, 2021

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-05-20 06:55 UTC