From: Oswald Buddenhagen Date: Thu, 23 Mar 2023 21:36:09 GMT Subject: Re: [PATCH 7/8] sequencer: pass `onto` to complete_action() as object-id Message-ID: In-Reply-To: On Thu, Mar 23, 2023 at 07:34:57PM +0000, Phillip Wood wrote: >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. >> 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?