Re: [PATCH 1/3] strbuf: make stripspace() part of strbuf
From: Tobias Klauser <tklauser@distanz.ch>
Date: 2016-06-15 23:06:53
Thanks for the review. On 2015-10-15 at 19:36:17 +0200, Junio C Hamano [off-list ref] wrote:
Tobias Klauser [off-list ref] writes:quoted
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_strbufI 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.
quoted
Signed-off-by: Tobias Klauser <tklauser@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