From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Wed, 29 Jul 2020 22:28:29 GMT
Subject: Re: [PATCH v5 31/39] bundle: add new version for use with SHA-256
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.

> 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