Subject: Re: [PATCHv2 1/3] format-patch: create patch filename in one function

From: Stephen Boyd <[email-masked]>
Date: Sat, 21 Mar 2009 22:59:24 -0700

Junio C Hamano wrote:
> The renaming is a good idea even without any change in the feature.
> Naming functions after what their result is used _for_ is never a good
> idea, and we should name them after what they do.
>
> Does it still make sense to pass "keep_subject" to the function?  After
> all what it does is to retain "[PATCH.." prefix that is useless for the
> purpose of making each patch easily identifiable.  Because people almost
> always use patch acceptance tools in non-keep mode to strip the
> "[PATCH..]"  prefix when creating the commits these days anyway, it may
> make more sense to lose the parameter altogether and simplify the
> processing.

I originally removed "keep_subject" but I took it out because I couldn't
figure out what it was being used for and I thought it might be
important. When it's removed all the tests pass. It's not even used by
other code besides a few if cases regarding numbering logic so I think
the entire command line switch could be removed.

>
> I also wonder if it makes sense to move what this function does into a
> user format; especially the logic that sanitizes the oneline string into
> filename friendly one may be something Porcelains may want an access to
> from outside.
>
> IOW, you can introduce a new format specifier (say, "%f") to
> format_commit_message() and the implemention of get_patch_filename() would
> just prepare a strbuf and call format_commit_message() on it, no?

This sounds great! I'm new so I don't know where to look for something
like this.