Thread (25 messages) 25 messages, 4 authors, 2016-06-15

Re: [PATCH v4 02/10] ref-filter: make the 'color' use ref_formatting_state

flat view

From: Karthik Nayak <hidden>
Date: 2016-06-15 23:05:53

On Sun, Jul 26, 2015 at 9:42 AM, Eric Sunshine [off-list ref] wrote:
Should 'color' should be declared 'char *' rather than 'const char *'?
It's always assigned via xstrdup(), and if declared 'char *', you
wouldn't have to cast away the 'const' when freeing it.
yes, will change.
While reviewing patch 1/10, it required more effort and was
distracting to have to separate out (mentally) changes which were
specific to the new %(align:X) pseudo-atom and those which introduced
the "formatting state". As such, it probably would be a good idea to
aim for the three distinct patches suggested by Junio rather than
aiming, up front, to merge the first two. After all, they are
conceptually distinct changes, and keeping them separate is friendlier
to reviewers. In the end, you may find that patch 1 is so trivial that
it can be merged with patch 2 without making review more difficult,
however, keeping them distinct during development helps you avoid
conflating unrelated changes.
Yes! I'm doing that like Junio suggested. Thanks for bringing it up :)

-- 
Regards,
Karthik Nayak
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help