MarcoFalke
commented at 4:04 PM on June 6, 2022:
member
This was done in the context of #25284 , but I think it also makes sense standalone.
The basic idea is that serialization type should not be initialized when it is not needed. Same for the serialization version.
So do this here for AutoFile. CAutoFile remains in places where it is not yet possible.
laanwj
commented at 4:36 PM on June 6, 2022:
member
Concept ACK
DrahtBot added the label Refactoring on Jun 6, 2022
DrahtBot
commented at 9:00 AM on June 7, 2022:
contributor
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Conflicts
No conflicts as of last run.
DrahtBot added the label Needs rebase on Jun 7, 2022
MarcoFalke force-pushed on Jun 7, 2022
DrahtBot removed the label Needs rebase on Jun 7, 2022
DrahtBot added the label Needs rebase on Jun 29, 2022
streams: Add AutoFile without ser-type and ser-version
The moved parts can be reviewed with "--color-moved=dimmed-zebra".
The one-char changes can be reviewed with "--word-diff-regex=.".
6666803c89
Use AutoFile where possiblefacc2fa7b8
MarcoFalke force-pushed on Jun 29, 2022
DrahtBot removed the label Needs rebase on Jun 29, 2022
laanwj
commented at 9:16 AM on June 29, 2022:
member
Code review ACKfacc2fa7b8a218a0df6a19772e1641ea68dda2e3
The code change is straightforward and easy to review.
However, I am not entirely happy about the naming of AutoFile versus CAutoFile I would personally prefer a more distinctive name.
MarcoFalke
commented at 9:26 AM on June 29, 2022:
member
However, I am not entirely happy about the naming of AutoFile versus CAutoFile I would personally prefer a more distinctive name.
CAutoFile is deprecated, only used in two remaining places: addrdb and tx/block serialization, and will be removed as a follow-up to #25284. So I left the name as is, but I am happy to append a scripted-diff here to rename CAutoFile to something else. Maybe LegacyAutoFile?
laanwj
commented at 10:04 AM on June 29, 2022:
member
OK, if it's going away soonâ„¢ i have no problem with this. There's something to be said for an intermediate rename, but also there's drawbacks in generating a bigger diff.
I just imagined it's pretty frustrating for developers to have similarly named classes with slightly different behavior, it's not something to keep on the long run (it reminds me of the situation in net classes naming CAddress / CService / CNetAddr a bit, where, even though i've worked on this code for years keep forgetting what is what).
MarcoFalke requested review from ryanofsky on Jul 19, 2022
fanquake approved
fanquake
commented at 8:31 AM on July 20, 2022:
member
ACKfacc2fa7b8a218a0df6a19772e1641ea68dda2e3
fanquake merged this on Jul 20, 2022
fanquake closed this on Jul 20, 2022
MarcoFalke deleted the branch on Jul 20, 2022
sidhujag referenced this in commit 9dfa0a3e11 on Jul 20, 2022
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-17 06:13 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me