Move src/tinyformat.h to src/tinyformat/tinyformat.h #13846
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
Member
|
Concept ACK |
Contributor
| No more conflicts as of last run. |
This was referenced Aug 2, 2018
Empact
force-pushed
the
tinyformat-subdir
branch
from
72e587b to
8b63251
Compare
This was referenced Aug 3, 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. |
Member
|
Reopening because of @MarcoFalke's concern, agree that #13845 pulls in a bizarrely high number (46!) of files. And the whole intent of |
Empact
force-pushed
the
tinyformat-subdir
branch
from
8b63251 to
8e60387
Compare
maflcko
reviewed
Aug 12, 2018
src/tinyformat/README.md
Outdated
Member
There was a problem hiding this comment.
No need to add a third party readme. This will show up in all history/logs and greps.
Member
|
Concept ACK |
Empact
force-pushed
the
tinyformat-subdir
branch
from
8e60387 to
c5ee680
Compare
Contributor
Author
|
Dropped the readme. 👍 |
maflcko
reviewed
Aug 27, 2018
test/lint/README.md
Outdated
Member
|
utACK c5ee680fdfe806ab6ffbfca47f6cd28e4fb4e049 |
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: 695041e - util: Update tinyformat 1b8fd35 - Make tinyformat errors raise an exception instead of assert()ing 9b6d4c5 - Move strprintf define to tinyformat.h 6e5fd00 - Move `*Version()` functions to version.h/cpp 9eaa0af - tinyformat: force USE_VARIADIC_TEMPLATES b651270 - util: Throw tinyformat::format_error on formatting error This does not include changes below, which are cosmetic or apparently unintentional: 64fb0ac - Declare single-argument (non-converting) constructors "explicit" 4d9b425 - 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.
Empact
force-pushed
the
tinyformat-subdir
branch
from
c5ee680 to
d48249e
Compare
Contributor
Author
|
Removed inaccurate test/lint/README mention and unnecessary test/lint/lint-includes.sh filter. |
Member
|
re-utACK d48249e |
Member
|
This was discussed at the 2018-08-30 IRC meeting and there was no agreement to do this at all. 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. |
Member
|
Imo this could still be done the next time we touch tinyformat, but yeah no rush |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
695041e - util: Update tinyformat
1b8fd35 - Make tinyformat errors raise an exception instead of assert()ing
9b6d4c5 - Move strprintf define to tinyformat.h
6e5fd00 - Move
*Version()functions to version.h/cpp9eaa0af - tinyformat: force USE_VARIADIC_TEMPLATES
b651270 - util: Throw tinyformat::format_error on formatting error
This does not include changes below, which are cosmetic or apparently unintentional:
64fb0ac - Declare single-argument (non-converting) constructors "explicit"
4d9b425 - 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.