On Sun, Jul 26, 2015 at 9:42 AM, Eric Sunshine <sunsh...@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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
