Re: [PATCH] Fix typos.

["Joseph S. Myers" <joseph at codesourcery dot com>]

On Sun, 18 Aug 2013, Ondrej Bilka wrote:

> ./ports/Changelog.aarch64:
> 
> 
> 	* bits/atomic.h: Fix typos.
> 	* strcmp.S: Likewise.
> 	* strlen.S: Likewise.
> 	* strnlen.S: Likewise.

Incorrect ChangeLog entries.  Paths given are always relative to the 
directory of the ChangeLog file, i.e. relative to the ports/ directory for 
all the ports/ ChangeLogs.

> ./ports/Changelog:

None of these belong in this file.  Those for architecture-specific files 
go in ChangeLog.<arch>.  Those for sysdeps/unix/sysv/linux/generic go in 
ChangeLog.linux-generic.

> e new larger chunk.  We then carry on \n-accreting characters to the end of the 
> e new larger chunk.  We then carry on \n+accrediting characters to the end of th
>                                               ^^                               

No, this change is wrong.  "accreting" is a valid English word and 
corresponds to the intended meaning here.

>  the result is denormal,  it will not \n-honour the double precision and generat
>  the result is denormal,  it will not \n+honor the double precision and generate
>                                             ^^                                 

Never mix changes that relate to choice of English language variant with 
actual typo fixes.  If a patch claims to fix typos, it should only contain 
changes that are completely unambiguously typo fixes.  Not changes of 
English language variant, not changes where you aren't sure of what was 
intended, not changes where colloquial usage in comments may use an 
abbreviated form such as "thru", or vary usage of spacing or hyphenation 
such as "bit field" / "bit-field" / "bitfield", or anything like that; 
just cases where the existing comment is simply unambiguously wrong rather 
than the writer's choice of informal usage.  Anything that could be 
considered stylistic needs discussing separately (and any standards in 
such cases are only really needed for the manual, not random comments).

>  /* Restore the default.  Better than \n-nother at all.  */ \n ctx->classes[0] =
>  /* Restore the default.  Better than \n+other at all.  */ \n ctx->classes[0] = 
>                                         ^^                                     

I don't think this comment really makes sense either before or after the 
change.  In such cases, a separate thread may be needed to discuss the 
intent of the comment.

>  data packet */ \n-#define ACK 04    /* acknowledgement */ \n #define ERROR 05  
>  data packet */ \n+#define ACK 04    /* acknowledgment */ \n #define ERROR 05   
>                                                  ^^                            

This is another English variant issue, not a typo issue.

>                              \n-/* Info abount the function itself.  */ \n n = p
>                              \n+/* Info amount the function itself.  */ \n n = p
>                                          ^                                     

No, "about", not "amount".

When sending typo fix patches you need to read every single modified 
comment yourself and satisfy yourself that the change does reflect the 
intent of the comment.  It's clear this has not been done in this case 
(other examples of changes that are simply wrong include "espects" -> 
"aspects", should be "expects", and "intrinsics" -> "intrinsic", when the 
original was correct).

I stopped commenting here.  For any subsequent revisions, in addition to 
the points I identified above, please keep patches down to at most 1000 
lines for convenience of review, and wait until consensus has been reached 
on a single posted patch before sending any subsequent patches (also of at 
most 1000 lines).

-- 
Joseph S. Myers
joseph@codesourcery.com