Re: [PATCH 7/8] sequencer: pass `onto` to complete_action() as object-id

From: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Date: Mar 23, 2023, 21:36 UTC
Message-ID: <ZBzGSbm7GZVK17ja@ugly>
In-Reply-To: <a3833d93-5db0-454e-526e-04681e5e5276@dunelm.org.uk>

On Thu, Mar 23, 2023 at 07:34:57PM +0000, Phillip Wood wrote:

Show 7 quoted lines

>On 23/03/2023 16:22, Oswald Buddenhagen wrote:
>> ... instead of as a commit, which makes the purpose clearer and will
>> simplify things later.
>
>given that we want onto to be a commit I'm not sure how this makes 
>anything clearer.
>

it makes it clearer that we need only the oid, not any other part of the commit object. and pulling ahead the "extraction" reduces the visual noise further down.

Show 7 quoted lines

>> As a side effect, this change revealed that skip_unnecessary_picks() was
>> butchering the commit object due to missing const-correctness. Slightly
>> adjust its API to rectify this.
>
>I don't think this is correct. If you look at the original code it makes 
>a copy of the oid and uses the copy when calling skip_unnecessary_picks()
>

oops, you're quite right. (facepalm) imo the change still makes sense, though, as it replaces the relatively expensive deep copies with simple pointer updates. so just fix the commit message?