Stephen Boyd <bebarino@xxxxxxxxx> writes: > reopen_stdout() usually takes the oneline description of a commit, > appends the patch suffix, prepends the output directory (if any) and > then reopens stdout as the resulting file. Now the patch filename (the > oneline description and the patch suffix) is created in > get_patch_filename() and passed to reopen_stdout() which prepends the > output directory and reopens stdout as that file. 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. > -static const char *get_oneline_for_filename(struct commit *commit, > - int keep_subject) > +static const char *get_patch_filename(char* sol, int keep_subject, int nr) Asterisk sticks to the variable name, not type name. 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? -- 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
