Re: [PATCH 1/3] pretty.c: rename describe options variable to more descriptive name
- From
Eli Schwartz <eschwartz@archlinux.org>
- Date
- Oct 24, 2021, 15:37 UTC
- Message-ID
- <bf4d8ed5-f168-6e9a-fb27-df735ff28eb6@archlinux.org>
- In-Reply-To
- <xmqqv91n6o1u.fsf@gitster.g>
On 10/24/21 12:31 AM, Junio C Hamano wrote:
Show 30 quoted lines
> Eli Schwartz <eschwartz@archlinux.org> writes:
>
>> It contains option arguments, not options. We would like to add option
>> support here too.
>>
>> Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
>> ---
>> pretty.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/pretty.c b/pretty.c
>> index 73b5ead509..9db2c65538 100644
>> --- a/pretty.c
>> +++ b/pretty.c
>> @@ -1216,7 +1216,7 @@ int format_set_trailers_options(struct process_trailer_options *opts,
>>
>> static size_t parse_describe_args(const char *start, struct strvec *args)
>> {
>> - const char *options[] = { "match", "exclude" };
>> + const char *option_arguments[] = { "match", "exclude" };
>
> This renaming is more or less "Meh" without the other change in the
> series that may (or may not) be helped with this step, but because I
> haven't seen these later steps yet, I may sound too dismissive of
> the change in this step.
>
> Anyway, at least call that option_args[] to match the way the
> function calls itself, not option_arguments[] that is a mouthful for
> a mere implementation detail of local variable for a file-private
> helper function.Right, the reason this change was submitted in its own patch was essentially for the sole purpose of making the diff for the second patch intuitive to read.
-- Eli Schwartz Arch Linux Bug Wrangler and Trusted User