Обсуждение: fix typo in reorderbuffer.c

Hi

I found a possible typo in reorderbuffer.c

 *   has got a combocid. Combocid's are only valid for the duration of a

Combocid's ==>> Combocids

Attatching a small patch to correct it.

Best regards,
houzj

> I found a possible typo in reorderbuffer.c
> 
>  *   has got a combocid. Combocid's are only valid for the duration
> of a
> 
> Combocid's ==>> Combocids
> 
> Attatching a small patch to correct it.
> 

Ping again, just in case it’s missed.

On Wed, Jan 27, 2021 at 05:57:06AM +0000, Hou, Zhijie wrote:
>> Combocid's ==>> Combocids
>
> Ping again, just in case it’s missed.

There are in the comments "ComboCIDs", "combocids" and "ComboCids" on
top of the existing Combocid's.  Your patch introduces a fourth way of
writing that.  It may be better to just have one way to govern them
all.
--
Michael

> >> Combocid's ==>> Combocids
> >
> > Ping again, just in case it’s missed.
> 
> There are in the comments "ComboCIDs", "combocids" and "ComboCids" on top
> of the existing Combocid's.  Your patch introduces a fourth way of writing
> that.  It may be better to just have one way to govern them all.

Hi Michael,

Thanks for the review.

Actually, "Combocid's" in the patch is the first word of the sentence, so the first char is uppercase(may be not a new
style).

I agree that it’s better to have a common way, but some different style of combocid follow the variable or
functionname[1].
We may need to change the var name or function name too.

Personally , I prefer "combocids".
But do you think we can change that in a separate patch apart from this typo patch ?


[1]:
void
SerializeComboCIDState(Size maxsize, char *start_address)

static int    usedComboCids = 0;    /* number of elements in comboCids */
...

Best regards,
houzj

On Wed, Jan 27, 2021 at 07:15:41AM +0000, Hou, Zhijie wrote:
> I agree that it’s better to have a common way, but some different
> style of combocid follow the variable or functionname[1].
> We may need to change the var name or function name too.
>
> [1]:
> void
> SerializeComboCIDState(Size maxsize, char *start_address)
>
> static int    usedComboCids = 0;    /* number of elements in comboCids */

Yes, the context matters a lot.  In the case you are quoting, it makes
sense to refer to the name of the variable in the comment.  If the
term is used in a comment with a more generic purpose, we should then
use a generic term.

> Personally , I prefer "combocids".
> But do you think we can change that in a separate patch apart from this typo patch ?

What about "Combo CID(s)", for Combo command ID?  README.parallel uses
this term for example.
--
Michael

> 
> What about "Combo CID(s)", for Combo command ID?  README.parallel uses
> this term for example.

Sorry for the late reply and yes, " Combo CID(s)" looks better.
Attaching patch which replaces all styles "Combocid(s)" with " Combo CID(s)".

Best regards,
houzj

On Wed, Mar 24, 2021 at 10:52:14AM +0000, houzj.fnst@fujitsu.com wrote:
> Sorry for the late reply and yes, " Combo CID(s)" looks better.
> Attaching patch which replaces all styles "Combocid(s)" with " Combo CID(s)".

I have double-checked the code, adjusted things a bit to adapt with
some of the context of the code, and applied it.  Thanks!
--
Michael