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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
