Re: [PATCH v5 31/39] bundle: add new version for use with SHA-256

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Jul 29, 2020, 22:28 UTC
Message-ID: <20200729222829.GF6540@camp.crustytoothpaste.net>
In-Reply-To: <CAPig+cSJc9P6hdBDrH9QbkvARcqc3EygMMXXB8vbU=W43viLqA@mail.gmail.com>

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.

Show 11 quoted lines

> 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