On 2020-07-29 at 02:53:47, Eric Sunshine wrote:
Moving the strbuf_rtrim() earlier in the loop, as suggested in my
previous review, made the diff a lot noisier, uglier, and more
difficult to read, however, the code itself ends up being easier to
reason about than in the previous round. Good.
Yeah, I noticed that. The code is much nicer, so thank you for that
suggestion.
By the way (I didn't think of this in my previous review), but
wouldn't it be better for this:
if (header->version == 3 && *buf.buf == '@') {
to instead be written as:
if (header->version >= 3 && *buf.buf == '@') {
to future-proof it since versions beyond 3 will also almost certainly
support "@foo" capabilities?
I think this will need to be dealt with by the author of bundle v4,
since currently bundle v4 will error out before we get here. I don't
want to make too many assumptions about a future format.
--
brian m. carlson: Houston, Texas, US