Hi John
On 14/03/2022 14:11, John Cai wrote:
Hi Junio,
On 13 Mar 2022, at 3:58, Junio C Hamano wrote:
"John Cai via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
diff --git a/reset.c b/reset.c
index e3383a93343..f8e32fcc240 100644
--- a/reset.c
+++ b/reset.c
@@ -101,6 +101,9 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
if (opts->branch_msg && !opts->branch)
BUG("branch reflog message given without a branch");
+ if (switch_to_branch && opts->flags & RESET_HEAD_DETACH)
It's just style thing but it probably is easier to read to have
an extra () around the bitwise-&.
+ BUG("attempting to detach HEAD when branch is given");
I wonder if there is a valid use case NOT to use RESET_HEAD_DETACH
when switch_to_branch == NULL. If there isn't, it could be that
we can get rid of RESET_HEAD_DETACH bit and base this decision
solely on switch_to_branch'es NULLness.
But that is totally outside the scope of this fix. I'd prefer to
see a narrowly and sharply focused fix, and to be quite honest, I
would be happier if this assertion weren't in this topic.
I'm okay with taking this out, but would be good to get an ack from Phillip.
That's fine with me. The rest of the patch looks good as far as I'm
concerned.
Best Wishes
Phillip
