public inbox for
 help / color / mirror / Atom feed
From: "Joseph S. Myers" <>
To: "Ondřej Bílka" <>
Cc: <>, <>
Subject: Re: [PATCH] Fix typos.
Date: Sun, 18 Aug 2013 20:15:00 -0000	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <20130818164943.GA7418@domone>

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 

> 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

  reply	other threads:[~2013-08-18 20:15 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-13  8:27 Ondřej Bílka
2013-08-14 19:18 ` Roland McGrath
2013-08-18 16:50 ` Ondřej Bílka
2013-08-18 20:15   ` Joseph S. Myers [this message]
2013-08-18 22:10     ` [PATCH 1/9] " Ondřej Bílka
2013-08-18 22:31       ` Allan McRae
2013-08-19 19:32       ` Joseph S. Myers
2013-08-21 10:38         ` [PATCH 1/9 v2] " Ondřej Bílka
2013-08-21 15:48           ` Joseph S. Myers
2013-08-22 13:56       ` [PATCH 2/9] " Ondřej Bílka
2013-08-22 15:04         ` Joseph S. Myers
2013-08-24  7:43           ` Ondřej Bílka
2013-08-28 16:07             ` Joseph S. Myers
2013-08-30 10:48       ` [PATCH 3/9] " Ondřej Bílka
2013-08-30 11:01         ` Will Newton
2013-08-30 15:47           ` [PATCH 3/9 v2] " Ondřej Bílka
2013-08-30 16:01             ` Joseph S. Myers
2013-08-30 12:58       ` [PATCH 3.5/9] Fix then/than typo Ondřej Bílka
2013-08-30 13:34         ` Allan McRae
2013-08-30 16:22         ` Brooks Moses
2013-09-02  9:30           ` [COMMITED] Fix additional typo Ondřej Bílka
2013-09-02  9:37       ` [PATCH 4/9] Fix typos Ondřej Bílka
2013-09-02 10:01         ` Florian Weimer
2013-09-02 10:15         ` Allan McRae

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).