Thread (1 message) 1 message, 1 author, 2017-10-21

Re: [PATCH 2/3] branch: split validate_new_branchname() into two

From: Junio C Hamano <hidden>
Date: 2017-10-21 09:02:03

Kaartic Sivaraam [off-list ref] writes:
quoted
+/*
+ * Check if a branch 'name' can be created as a new branch; die otherwise.
+ * 'force' can be used when it is OK for the named branch already exists.
+ * Return 1 if the named branch already exists; return 0 otherwise.
+ * Fill ref with the full refname for the branch.
+ */
I guess it's better to avoid repeating the comments in both the .h and
.c file as they might quite easily become stale. I would prefer keeping
it in the header file alone.
True.  I wrote this while designing the code, so the copy in .c file
came first, and then that was copied to .h file; the one in .c file
can go.
The change was simple by splitting the function into two and calling
the right function as required at every call site! As far as I could
see this doesn't seem to be reducing the confusion that the 'attr_only'
parameter caused.
The confusion primarily was the way the parameter was named.
"forcing" does not have strong connection to marking "this is only
asking about attributes".  And removing that confusion, by
separating the caller and making it explicit that the caller needs
two separate behaviours depending on the condition, and naming the
functions more appropriately (i.e. is this about creating a new one
that must not exist already?), is the focus of this step.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help