Thread (2 messages) flat view 2 messages, 2 authors, 2016-06-15

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_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.
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help