Re: [PATCH] Lengthening FORMAT_PATCH_NAME_MAX to 80
From: Junio C Hamano <hidden>
Date: 2020-11-06 17:45:16
hukeping [off-list ref] writes:
quoted
I do not mind getting rid of the "FORMAT_PATCH_NAME_MAX" constant and replacing it with a variable that defaults to 64 and can be tweaked by a command line option and/or a configuration variable. It does not feel it is worth the effort to replace one hardcoded constant with another hardcoded constant.quoted
Looking at the code which uses the constant, I suspect it could also be made simpler: - the PATH_MAX check in open_next_file() seems pointless. Once upon a time it mattered for fitting into a PATH_MAX buffer, but these days we use a dynamic buffer anyway. We are probably better off to just feed the result to the filesystem and see if it complains (since either way we are aborting; I'd feel differently if we adjusted our truncation size) - the logic in fmt_output_subject() could probably be simpler if the constant was "here's how long the subject should be", not "here's how long the whole thing must be". But those are both orthogonal to your patch and can be done separately.Yes, these clean-ups seem worth doing.Agreed, and I'd like to do it with two separated commits: - commit-1, cleanup the open_next_file() by drop the if (filename.len>=..) statements. - commit-2, replace FORMAT_PATCH_NAME_MAX in fmt_output_subject() with a constant in there and make it to 80(or other value?), and drop FORMAT_PATCH_NAME_MAX from log-tree.h. Is this works for you?
I am not sure what you meant by "Agreed". I said two things: - It is dubious that it is worth the effort to replace a hardcoded constant with another. Making it configurable with command line option and/or configuration variable may be worth doing. - Two observations Peff made for further clean-up are probably worth doing. If you are agreeing to both of the above and following through, then yes, it seems like a good plan. If agreement is only to the former, it probably still is worth doing. Anything else, I don't know.