Replace l337sp34k in comments.

Started by Peter Smithover 4 years ago15 messages
#1Peter Smith
smithpb2250@gmail.com
1 attachment(s)

IMO the PG code comments are not an appropriate place for leetspeak creativity.

PSA a patch to replace a few examples that I recently noticed.

"up2date" --> "up-to-date"

------
Kind Regards,
Peter Smith.
Fujitsu Australia

Attachments:

v1-0001-Replace-leetspeak-comments-with-English.patchapplication/octet-stream; name=v1-0001-Replace-leetspeak-comments-with-English.patchDownload
From caa69865171ee8b21348e1dc2f1cbdff8e96dd69 Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Wed, 28 Jul 2021 09:19:25 +1000
Subject: [PATCH v1] Replace leetspeak comments with English

---
 src/backend/jit/llvm/llvmjit_expr.c       |  2 +-
 src/backend/replication/logical/logical.c | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index 8a4075b..6d11812 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -2386,7 +2386,7 @@ llvm_compile_expr(ExprState *state)
  * Run compiled expression.
  *
  * This will only be called the first time a JITed expression is called. We
- * first make sure the expression is still up2date, and then get a pointer to
+ * first make sure the expression is still up-to-date, and then get a pointer to
  * the emitted function. The latter can be the first thing that triggers
  * optimizing and emitting all the generated functions.
  */
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index d61ef4c..42f47fd 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -1048,7 +1048,7 @@ change_cb_wrapper(ReorderBuffer *cache, ReorderBufferTXN *txn,
 	ctx->write_xid = txn->xid;
 
 	/*
-	 * report this change's lsn so replies from clients can give an up2date
+	 * report this change's lsn so replies from clients can give an up-to-date
 	 * answer. This won't ever be enough (and shouldn't be!) to confirm
 	 * receipt of this transaction, but it might allow another transaction's
 	 * commit to be confirmed with one message.
@@ -1088,7 +1088,7 @@ truncate_cb_wrapper(ReorderBuffer *cache, ReorderBufferTXN *txn,
 	ctx->write_xid = txn->xid;
 
 	/*
-	 * report this change's lsn so replies from clients can give an up2date
+	 * report this change's lsn so replies from clients can give an up-to-date
 	 * answer. This won't ever be enough (and shouldn't be!) to confirm
 	 * receipt of this transaction, but it might allow another transaction's
 	 * commit to be confirmed with one message.
@@ -1225,7 +1225,7 @@ stream_start_cb_wrapper(ReorderBuffer *cache, ReorderBufferTXN *txn,
 	ctx->write_xid = txn->xid;
 
 	/*
-	 * report this message's lsn so replies from clients can give an up2date
+	 * report this message's lsn so replies from clients can give an up-to-date
 	 * answer. This won't ever be enough (and shouldn't be!) to confirm
 	 * receipt of this transaction, but it might allow another transaction's
 	 * commit to be confirmed with one message.
@@ -1272,7 +1272,7 @@ stream_stop_cb_wrapper(ReorderBuffer *cache, ReorderBufferTXN *txn,
 	ctx->write_xid = txn->xid;
 
 	/*
-	 * report this message's lsn so replies from clients can give an up2date
+	 * report this message's lsn so replies from clients can give an up-to-date
 	 * answer. This won't ever be enough (and shouldn't be!) to confirm
 	 * receipt of this transaction, but it might allow another transaction's
 	 * commit to be confirmed with one message.
@@ -1443,7 +1443,7 @@ stream_change_cb_wrapper(ReorderBuffer *cache, ReorderBufferTXN *txn,
 	ctx->write_xid = txn->xid;
 
 	/*
-	 * report this change's lsn so replies from clients can give an up2date
+	 * report this change's lsn so replies from clients can give an up-to-date
 	 * answer. This won't ever be enough (and shouldn't be!) to confirm
 	 * receipt of this transaction, but it might allow another transaction's
 	 * commit to be confirmed with one message.
@@ -1535,7 +1535,7 @@ stream_truncate_cb_wrapper(ReorderBuffer *cache, ReorderBufferTXN *txn,
 	ctx->write_xid = txn->xid;
 
 	/*
-	 * report this change's lsn so replies from clients can give an up2date
+	 * report this change's lsn so replies from clients can give an up-to-date
 	 * answer. This won't ever be enough (and shouldn't be!) to confirm
 	 * receipt of this transaction, but it might allow another transaction's
 	 * commit to be confirmed with one message.
-- 
1.8.3.1

#2Michael Paquier
michael@paquier.xyz
In reply to: Peter Smith (#1)
Re: Replace l337sp34k in comments.

On Wed, Jul 28, 2021 at 09:39:02AM +1000, Peter Smith wrote:

IMO the PG code comments are not an appropriate place for leetspeak creativity.

PSA a patch to replace a few examples that I recently noticed.

"up2date" --> "up-to-date"

Agreed that this is a bit cleaner to read, so done. Just note that
pgindent has been complaining about the format of some of the updated
comments.
--
Michael

#3Peter Smith
smithpb2250@gmail.com
In reply to: Michael Paquier (#2)
Re: Replace l337sp34k in comments.

On Wed, Jul 28, 2021 at 11:32 AM Michael Paquier <michael@paquier.xyz> wrote:

On Wed, Jul 28, 2021 at 09:39:02AM +1000, Peter Smith wrote:

IMO the PG code comments are not an appropriate place for leetspeak creativity.

PSA a patch to replace a few examples that I recently noticed.

"up2date" --> "up-to-date"

Agreed that this is a bit cleaner to read, so done. Just note that
pgindent has been complaining about the format of some of the updated
comments.

Thanks for pushing!

BTW, the commit comment [1]https://github.com/postgres/postgres/commit/7b7fbe1e8bb4b2a244d1faa618789db411316e55 attributes most of these to a recent
patch, but I think that is mistaken. AFAIK they are from when the
file was first introduced 8 years ago [2]https://github.com/postgres/postgres/commit/b89e151054a05f0f6d356ca52e3b725dd0505e53#diff-034b6d4eaf36425e75d7a7087d09bd6c734dd9ea8398533559d537d13b6b9197.

------
[1]: https://github.com/postgres/postgres/commit/7b7fbe1e8bb4b2a244d1faa618789db411316e55
[2]: https://github.com/postgres/postgres/commit/b89e151054a05f0f6d356ca52e3b725dd0505e53#diff-034b6d4eaf36425e75d7a7087d09bd6c734dd9ea8398533559d537d13b6b9197

Kind Regards,
Peter Smith.
Fujitsu Australia

#4Andrew Dunstan
andrew@dunslane.net
In reply to: Peter Smith (#1)
Re: Replace l337sp34k in comments.

On 7/27/21 7:39 PM, Peter Smith wrote:

IMO the PG code comments are not an appropriate place for leetspeak creativity.

PSA a patch to replace a few examples that I recently noticed.

"up2date" --> "up-to-date"

Personally, I would have written this as just "up to date", I don't
think the hyphens are required.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

#5Geoff Winkless
pgsqladmin@geoff.dj
In reply to: Andrew Dunstan (#4)
Re: Replace l337sp34k in comments.

On Thu, 29 Jul 2021 at 11:22, Andrew Dunstan <andrew@dunslane.net> wrote:

Personally, I would have written this as just "up to date", I don't
think the hyphens are required.

FWIW Mirriam-Webster and the CED suggest "up-to-date" when before a noun,
so the changes should be "up-to-date answer" but "are up to date".

https://dictionary.cambridge.org/dictionary/english/up-to-date

Geoff

#6Andrew Dunstan
andrew@dunslane.net
In reply to: Geoff Winkless (#5)
Re: Replace l337sp34k in comments.

On 7/29/21 8:51 AM, Geoff Winkless wrote:

On Thu, 29 Jul 2021 at 11:22, Andrew Dunstan <andrew@dunslane.net
<mailto:andrew@dunslane.net>> wrote:

Personally, I would have written this as just "up to date", I don't
think the hyphens are required.

 
FWIW Mirriam-Webster and the CED suggest "up-to-date" when before a
noun, so the changes should be "up-to-date answer" but "are up to date".

https://dictionary.cambridge.org/dictionary/english/up-to-date
<https://dictionary.cambridge.org/dictionary/english/up-to-date&gt;

Interesting, thanks. My (admittedly old) Concise OED only has the
version with spaces, while my (also old) Collins Concise has the
hyphenated version. I learn something new every day, no matter how trivial.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

#7Gavin Flower
GavinFlower@archidevsys.co.nz
In reply to: Geoff Winkless (#5)
Re: Replace l337sp34k in comments.

On 30/07/21 12:51 am, Geoff Winkless wrote:

On Thu, 29 Jul 2021 at 11:22, Andrew Dunstan <andrew@dunslane.net
<mailto:andrew@dunslane.net>> wrote:

Personally, I would have written this as just "up to date", I don't
think the hyphens are required.

FWIW Mirriam-Webster and the CED suggest "up-to-date" when before a
noun, so the changes should be "up-to-date answer" but "are up to date".

https://dictionary.cambridge.org/dictionary/english/up-to-date
<https://dictionary.cambridge.org/dictionary/english/up-to-date&gt;

Geoff

That 'feels' right to me.

Though in code, possibly it would be better to just use 'up-to-date' in
code for consistency and to make the it easier to grep?

As a minor aside: double quotes should be used for speech and single
quotes for quoting!

Cheers,
Gavin

#8Michael Paquier
michael@paquier.xyz
In reply to: Gavin Flower (#7)
Re: Replace l337sp34k in comments.

On Fri, Jul 30, 2021 at 09:46:59AM +1200, Gavin Flower wrote:

That 'feels' right to me.

Though in code, possibly it would be better to just use 'up-to-date' in code
for consistency and to make the it easier to grep?

The change in llvmjit_expr.c may not look like an adjective though,
which I admit can be a bit confusing. Still that does not look
completely wrong to me either.
--
Michael

#9Geoff Winkless
pgsqladmin@geoff.dj
In reply to: Gavin Flower (#7)
Re: Replace l337sp34k in comments.

On Thu, 29 Jul 2021 at 22:46, Gavin Flower
<GavinFlower@archidevsys.co.nz> wrote:

Though in code, possibly it would be better to just use 'up-to-date' in
code for consistency and to make the it easier to grep?

If it's causing an issue, perhaps using a less syntactically
problematic synonym like "current" might be better?

:)

Geoff

#10Gavin Flower
GavinFlower@archidevsys.co.nz
In reply to: Geoff Winkless (#9)
Re: Replace l337sp34k in comments.

On 30/07/21 8:05 pm, Geoff Winkless wrote:

On Thu, 29 Jul 2021 at 22:46, Gavin Flower
<GavinFlower@archidevsys.co.nz> wrote:

Though in code, possibly it would be better to just use 'up-to-date' in
code for consistency and to make the it easier to grep?

If it's causing an issue, perhaps using a less syntactically
problematic synonym like "current" might be better?

:)

Geoff

On thinking further...

The word 'current' means different things in different contexts. If I
refer to my current O/S it means the one I'm using now, but it may not
be current.  The second use of 'current' is the meaning you are thinking
of, but the first is not. Since people reading documented code are
focused on understanding technical aspects, they may miss this subtlety.

I'm aware that standardisation may meet with some resistance, but being
consistent might reduce the conceptual impedance when reading the code. 
I'm just trying to reduce the potential for confusion.

Cheers,
Gavin

#11Andrey Borodin
x4mmm@yandex-team.ru
In reply to: Peter Smith (#1)
Re: Replace l337sp34k in comments.

28 июля 2021 г., в 04:39, Peter Smith <smithpb2250@gmail.com> написал(а):

IMO the PG code comments are not an appropriate place for leetspeak creativity.

PSA a patch to replace a few examples that I recently noticed.

"up2date" --> "up-to-date"

FWIW, my 2 cents.
I do not see much difference between up2date, up-to-date, up to date, current, recent, actual, last, newest, correct, fresh etc.
I'm slightly leaning to 1337 version, but this can totally be ignored.

As a non-native speaker I'm a bit concerned by the fact that comment is copied 6 times. For me it's not a single bit easier to read comment then code. If this comment is that important, maybe refactor this assignment into function and document once?

Thanks!

Best regards, Andrey Borodin.

In reply to: Andrey Borodin (#11)
Re: Replace l337sp34k in comments.

On Sat, Jul 31, 2021 at 11:22 AM Andrey Borodin <x4mmm@yandex-team.ru> wrote:

FWIW, my 2 cents.
I do not see much difference between up2date, up-to-date, up to date, current, recent, actual, last, newest, correct, fresh etc.

+1.

To me it seems normal to debate wording/terminology with new code
comments, but that's about it. I find this zeal to change old code
comments misguided. It's okay if they're clearly wrong or have typos.
Anything else is just hypercorrection. And in any case there is a very
real chance of making the overall situation worse rather than better.
Probably in some subtle but important way.

See also: commit 8a47b775a16fb4f1e154c0f319a030498e123164

--
Peter Geoghegan

#13David G. Johnston
david.g.johnston@gmail.com
In reply to: Peter Geoghegan (#12)
Re: Replace l337sp34k in comments.

On Saturday, July 31, 2021, Peter Geoghegan <pg@bowt.ie> wrote:

On Sat, Jul 31, 2021 at 11:22 AM Andrey Borodin <x4mmm@yandex-team.ru>
wrote:

FWIW, my 2 cents.
I do not see much difference between up2date, up-to-date, up to date,

current, recent, actual, last, newest, correct, fresh etc.

+1.

To me it seems normal to debate wording/terminology with new code
comments, but that's about it. I find this zeal to change old code
comments misguided.

Maybe in general I would agree but I agree that this warrants an
exception. While maybe not explicitly stated the use of up2date as a term
is against the de-facto style guide for our project and should be corrected
regardless of how long it took to discover the violation. We fix other
unimportant but obvious typos all the time and this is no different. We
don’t ask people to police this but we also don’t turn down well-written
patches.

David J.

#14Andres Freund
andres@anarazel.de
In reply to: Peter Geoghegan (#12)
Re: Replace l337sp34k in comments.

Hi,

On 2021-07-31 12:15:34 +0300, Peter Geoghegan wrote:

On Sat, Jul 31, 2021 at 11:22 AM Andrey Borodin <x4mmm@yandex-team.ru> wrote:

FWIW, my 2 cents.
I do not see much difference between up2date, up-to-date, up to date, current, recent, actual, last, newest, correct, fresh etc.

+1.

To me it seems normal to debate wording/terminology with new code
comments, but that's about it. I find this zeal to change old code
comments misguided. It's okay if they're clearly wrong or have typos.
Anything else is just hypercorrection. And in any case there is a very
real chance of making the overall situation worse rather than better.
Probably in some subtle but important way.

Same here. I find them quite distracting, even.

It's one thing for such patches to target blindly obvious typos etc, but
they often also end up including less clear cut changes, which cost a
fair bit of time to review/judge.

Greetings,

Andres Freund

#15Andrew Dunstan
andrew@dunslane.net
In reply to: Andres Freund (#14)
Re: Replace l337sp34k in comments.

On 8/1/21 5:10 PM, Andres Freund wrote:

Hi,

On 2021-07-31 12:15:34 +0300, Peter Geoghegan wrote:

On Sat, Jul 31, 2021 at 11:22 AM Andrey Borodin <x4mmm@yandex-team.ru> wrote:

FWIW, my 2 cents.
I do not see much difference between up2date, up-to-date, up to date, current, recent, actual, last, newest, correct, fresh etc.

+1.
To me it seems normal to debate wording/terminology with new code
comments, but that's about it. I find this zeal to change old code
comments misguided. It's okay if they're clearly wrong or have typos.
Anything else is just hypercorrection. And in any case there is a very
real chance of making the overall situation worse rather than better.
Probably in some subtle but important way.

Same here. I find them quite distracting, even.

It's one thing for such patches to target blindly obvious typos etc, but
they often also end up including less clear cut changes, which cost a
fair bit of time to review/judge.

I agree. Errors, ambiguities and typos should be fixed, but purely
stylistic changes should not be made. In any case, I don't think we need
to hold the code comments to the same standard as the docs. I think a
little more informality is acceptable in code comments.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com