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

[Karthik Nayak <karthik.188@gmail.com>]

On Sun, Jul 26, 2015 at 9:42 AM, Eric Sunshine <sunshine@sunshineco.com> 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