Re: [RFC PATCH 5/9] sequencer: use const variable for commit message comments

Taylor Blau [off-list ref] writes:

Two nit-picks here. The line break after the '=' is a little awkward,
since two of these lines are less than 80 characters combined, and the
other two are just slightly longer than 80 characters. The other nitpick
is that its typical to see 'char *foo' instead of 'char foo[]'.

So, I'd write these as:

    static const char *first_commit_msg_str = N_("This is the 1st commit message:");
    static const char *nth_commit_msg_fmt = N_("This is the commit message #%d:");
    static const char *skip_nth_commit_msg_fmt = N_("The commit message #%d will be skipped:");
    static const char *combined_commit_msg_str = N_("This is a combination of %d commits.");

I actually am OK with []; it saves sizeof(char*) bytes from each of
the variable, doesn't it?

I also noticed that you suffix these with _fmt or _str depending on
whether or not there are arguments that get filled in later on. That
makes 'combined_commit_msg_str' labeled incorrectly (it should be
'combined_commit_msg_fmt').

Good eyes.

Thanks.

On Wed, Jan 13, 2021 at 9:37 PM Junio C Hamano [off-list ref] wrote:

Taylor Blau [off-list ref] writes:

[...]

quoted

The other nitpick
is that its typical to see 'char *foo' instead of 'char foo[]'.

So, I'd write these as:

    static const char *first_commit_msg_str = N_("This is the 1st commit message:");
    static const char *nth_commit_msg_fmt = N_("This is the commit message #%d:");
    static const char *skip_nth_commit_msg_fmt = N_("The commit message #%d will be skipped:");
    static const char *combined_commit_msg_str = N_("This is a combination of %d commits.");

I actually am OK with []; it saves sizeof(char*) bytes from each of
the variable, doesn't it?

Yeah, that's my understanding too.