Re: [PATCH v2 03/24] refs: convert delete_ref and refs_delete_ref to struct object_id
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Oct 9, 2017, 22:43 UTC
- Message-ID
- <20171009224309.GG19555@aiede.mtv.corp.google.com>
- In-Reply-To
- <20171009011132.675341-4-sandals@crustytoothpaste.net>
brian m. carlson wrote:
Show 16 quoted lines
> Convert delete_ref and refs_delete_ref to take a pointer to struct > object_id. Update the documentation accordingly, including referring to > null_oid in lowercase, as it is not a #define constant. > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> > --- > builtin/branch.c | 2 +- > builtin/replace.c | 2 +- > builtin/reset.c | 2 +- > builtin/tag.c | 2 +- > builtin/update-ref.c | 2 +- > refs.c | 21 +++++++++++---------- > refs.h | 12 ++++++------ > refs/files-backend.c | 2 +- > t/helper/test-ref-store.c | 6 +++--- > 9 files changed, 26 insertions(+), 25 deletions(-)
Was this prepared using coccinelle? (Just curious.)
[...]
Show 14 quoted lines
> @@ -663,12 +663,13 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,
>
> if (ref_type(refname) == REF_TYPE_PSEUDOREF) {
> assert(refs == get_main_ref_store());
> - return delete_pseudoref(refname, old_sha1);
> + return delete_pseudoref(refname, old_oid);
> }
>
> transaction = ref_store_transaction_begin(refs, &err);
> if (!transaction ||
> - ref_transaction_delete(transaction, refname, old_sha1,
> + ref_transaction_delete(transaction, refname,
> + old_oid ? old_oid->hash : NULL,
> flags, msg, &err) ||musing out loud: Distinguishing contexts where we need this kind of change and contexts where we can use old_oid->hash directly can be subtle. Do we need some kind of annotation to mark which parameters are nullable and which aren't?
[...]
Show 18 quoted lines
> --- a/refs.h > +++ b/refs.h > @@ -371,19 +371,19 @@ int refs_reflog_exists(struct ref_store *refs, const char *refname); > int reflog_exists(const char *refname); > > /* > - * Delete the specified reference. If old_sha1 is non-NULL, then > + * Delete the specified reference. If old_oid is non-NULL, then > * verify that the current value of the reference is old_sha1 before > - * deleting it. If old_sha1 is NULL, delete the reference if it > - * exists, regardless of its old value. It is an error for old_sha1 to > - * be NULL_SHA1. msg and flags are passed through to > + * deleting it. If old_oid is NULL, delete the reference if it > + * exists, regardless of its old value. It is an error for old_oid to > + * be null_oid. msg and flags are passed through to > * ref_transaction_delete(). > */ > int refs_delete_ref(struct ref_store *refs, const char *msg,
Thanks for updating the comment.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>