Re: [PATCH v5 6/7] refs: rename `refs_create_symref()` to `refs_update_symref()`
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- May 2, 2024, 11:34 UTC
- Message-ID
- <CAOLa=ZSJABxcW643hmpsot5M-_-h-_XHpNAgoof9HCXwj1zhKQ@mail.gmail.com>
- In-Reply-To
- <ZjNFFreAkolYMCd_@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
Show 20 quoted lines
> On Wed, May 01, 2024 at 10:22:28PM +0200, Karthik Nayak wrote: >> From: Karthik Nayak <karthik.188@gmail.com> >> >> The `refs_create_symref()` function is used to update/create a symref. >> But it doesn't check the old target of the symref, if existing. It force >> updates the symref. In this regard, the name `refs_create_symref()` is a >> bit misleading. So let's rename it to `refs_update_symref()`. This is >> akin to how 'git-update-ref(1)' also allows us to create apart from >> update. > > Arguably, as we are already updating all callsites anyway, I don't see a > reason why we shouldn't also update the function to accept the old OID > or old target so that callers can make raceless updates. > > The only problem is that we don't have any users yet, and consequently > we have no way to verify that it works as intended. So maybe this is > better left for a future patch series, unless we have places where we > can reasonably update the callers to pass in the old value, as well. > > Patrick
Yeah that did run through my mind too, but without a usecase, its hard to justify a change and also like you mentioned harder to write tests for.