Thanks for the review. On 2015-10-15 at 19:36:17 +0200, Junio C Hamano <gits...@pobox.com> wrote: > Tobias Klauser <tklau...@distanz.ch> writes: > > > Rename stripspace() to strbuf_stripspace() and move it to the strbuf > > module as suggested in [1]. > > > > Also switch all current users of stripspace() to the new function name > > and keep a temporary wrapper inline function for topic branches still > > using stripspace(). > > > > [1] > > https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#make_.27stripspace.28.29.27_part_of_strbuf > > I think Matthieu already mentioned this, but please explain why it > is a good idea to do this in your own words here, without forcing > readers to go to other places to find out. Giving credit to an > external page for giving you an inspiration to do something is good, > but the proposed log message needs to justify itself. In other > words, when you are challenged to defend this change, you are not > allowed to say "SmallProjectIdeas page said it is a good thing to > do. I just did it without questioning it." ;-) Instead you are > expected to justify it yourself.
Yes, makes sense. I'll adjust the commit message for v2 to justify the change for itself and move the link below --- as Matthieu suggested. > > Signed-off-by: Tobias Klauser <tklau...@distanz.ch> > > --- > > A good rule of thumb to see if it is a good time to do this kind of > change is to do this: > > $ git log -p maint..pu | grep stripspace > > which shows only one mention in a context, so this change may cause > textual conflict with something else somewhere nearby but won't hurt > other topics in flight. Ok, thanks for the hint. I'll check again before resubmitting v2. Thanks -- 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
