On Mon, Mar 24, 2025 at 02:50:56PM +0000, Karthik Nayak wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
>
> > On Thu, Mar 20, 2025 at 12:44:00PM +0100, Karthik Nayak wrote:
> >> diff --git a/refs.h b/refs.h
> >> index 240e2d8537..dcd83e81e2 100644
> >> --- a/refs.h
> >> +++ b/refs.h
> >> @@ -16,6 +16,29 @@ struct worktree;
> >> enum ref_storage_format ref_storage_format_by_name(const char *name);
> >> const char *ref_storage_format_to_name(enum ref_storage_format ref_storage_format);
> >>
> >> +/*
> >> + * enum ref_transaction_error represents the following return codes:
> >> + * REF_TRANSACTION_ERROR_GENERIC error_code: default error code.
> >> + * REF_TRANSACTION_ERROR_NAME_CONFLICT error_code: ref name conflict like A vs A/B.
> >> + * REF_TRANSACTION_ERROR_CREATE_EXISTS error_code: ref to be created already exists.
> >> + * REF_TRANSACTION_ERROR_NONEXISTENT_REF error_code: ref expected but doesn't exist.
> >> + * REF_TRANSACTION_ERROR_INCORRECT_OLD_VALUE error_code: provided old_oid or old_target of
> >> + * reference doesn't match actual.
> >> + * REF_TRANSACTION_ERROR_INVALID_NEW_VALUE error_code: provided new_oid or new_target is
> >> + * invalid.
> >> + * REF_TRANSACTION_ERROR_EXPECTED_SYMREF error_code: expected ref to be symref, but is a
> >> + * regular ref.
> >> + */
> >> +enum ref_transaction_error {
> >> + REF_TRANSACTION_ERROR_GENERIC = -1,
> >> + REF_TRANSACTION_ERROR_NAME_CONFLICT = -2,
> >> + REF_TRANSACTION_ERROR_CREATE_EXISTS = -3,
> >> + REF_TRANSACTION_ERROR_NONEXISTENT_REF = -4,
> >> + REF_TRANSACTION_ERROR_INCORRECT_OLD_VALUE = -5,
> >> + REF_TRANSACTION_ERROR_INVALID_NEW_VALUE = -6,
> >> + REF_TRANSACTION_ERROR_EXPECTED_SYMREF = -7,
> >> +};
> >> +
> >
> > Tiny nit: I think it's generally preferable to document each specific
> > right next to its definition.
> >
>
> Idk about this, I based it off on `enum bisect_error` which is similar,
> but I also see the same in `enum scld_error`.
>
> I'm okay to change this though, perhaps makes sense to document the
> preferred style however.
Probably would make sense, yes. The argument for why it's preferable to
do this inline is that it's harder for the list to grow stale. It's
trivial to see when a comment needs to be removed/updated.
Patrick
