Re: [PATCH v2 0/7] Improved infrastructure for refname normalization

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v2 0/7] Improved infrastructure for refname normalization

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:52:01

Michael Haggerty [off-list ref] writes:
Patch series re-roll:
Thanks for working on this. I very much like the general direction of the
series, the strategy to avoid wholesale audit of the callers and marking
the places that needs fixing with "_unsafe()".

There were a few minor things that looked worth mentioning while
reviewing, though.

 - (style) You seem to be fond of pre-increment a lot, but in general our
   codebase prefers post-increment especially when the end result does not
   make any difference, e.g.

	for (i = 1; ...; ++i) {
        	...

 - (series structure) It might make the series progress easier to follow
   if you introduced check_ref_format_unsafe() in the same commit where
   you change check_ref_format() to take flags parameter.

Re: [PATCH v2 0/7] Improved infrastructure for refname normalization

From: Michael Haggerty <hidden>
Date: 2016-06-15 22:52:01

On 09/12/2011 06:28 AM, Junio C Hamano wrote:
Michael Haggerty [off-list ref] writes:
There were a few minor things that looked worth mentioning while
reviewing, though.

 - (style) You seem to be fond of pre-increment a lot, but in general our
   codebase prefers post-increment especially when the end result does not
   make any difference, e.g.

	for (i = 1; ...; ++i) {
        	...
OK, changed.
 - (series structure) It might make the series progress easier to follow
   if you introduced check_ref_format_unsafe() in the same commit where
   you change check_ref_format() to take flags parameter.
OK.  I'll take the opportunity to rename the functions to
check_refname_format*(), to make it more obvious that they only concern
themselves with the refnames and not the references themselves.

I discovered a bug in my code for handling refnames without
normalization; I will also fix that in v3.

OTOH I am again having serious doubts that trying to support
unnormalized refnames is a good idea.  I will write more when I have
time to argue my case.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help