doc: provide context for CNetAddr::UnserializeV1Array() and span.h with lifetimebound #22881

pull jonatack wants to merge 2 commits into bitcoin:master from jonatack:span-lifetimebound changing 2 files +12 −2

jonatack commented at 6:32 PM on September 3, 2021: member
Add contextual documentation for developers and future readers of the code regarding
- CNetAddr::UnserializeV1Array (see #22140)
- Span and why it defines Clang lifetimebound locally rather than using the one in attributes.h
jonatack commented at 6:41 PM on September 3, 2021: member

I just saw this explanatory comment at the time: https://github.com/bitcoin/bitcoin/pull/19387/files#r451060526. Not sure if the context is still the same, as the attributes header file previously contained other code.
sipa commented at 6:43 PM on September 3, 2021: member

Yeah, this was done intentionally to keep span.h reusable without depending on other project files. I don't think anything changed about that.

I don't personally feel strongly either way; just pointing out the reason back then.
DrahtBot added the label Refactoring on Sep 3, 2021
practicalswift commented at 1:41 PM on September 4, 2021: contributor

cr ACK 419d4767d07f0d9affddca6bb8c469ae79a47d63
fanquake commented at 11:22 AM on September 5, 2021: member

cc @theuni
jonatack commented at 11:29 AM on September 5, 2021: member

(If it is preferred to keep span.h self-contained, I'd be tempted to convert this patch to documenting the intention and reason here and wrt #22140 to help future readers of the code.)
laanwj commented at 11:22 AM on September 9, 2021: member

No strong opinion here, remember that in principle the entire file can be considered temporary compatibility code until c++20, it's fine with me to treat it as a self-contained drop-in instead of really a part of our project.
net, doc: provide context for UnserializeV1Array() d14395bc5d
jonatack renamed this:
~~refactoring: use attributes::LIFETIMEBOUND in span.h~~
doc: provide context for CNetAddr::UnserializeV1Array() and span.h with lifetimebound
on Sep 23, 2021
span, doc: provide span.h context and explain lifetimebound definition 33c6a208a9
jonatack force-pushed on Sep 23, 2021
jonatack commented at 3:54 PM on September 23, 2021: member
Updated this patch to provide contextual documentation for developers regarding
- CNetAddr::UnserializeV1Array, e.g. #22140
- Span and why it defines Clang lifetimebound locally rather than using the one in attributes.h
laanwj commented at 4:06 PM on November 17, 2021: member

Thanks for the clarifications. Documentation review ACK 33c6a208a9e2512a174c99c224a933a59f091bc2
laanwj merged this on Nov 17, 2021
laanwj closed this on Nov 17, 2021
jonatack deleted the branch on Nov 17, 2021
sidhujag referenced this in commit 675133c862 on Nov 17, 2021
DrahtBot locked this on Nov 17, 2022

Contributors

Labels

Linked (view graph)

#22140 p2p, refactor: remove unneeded CNetAddr::UnserializeV1Array()