Docs: Always use true|false instead of sometimes on|off for the subscription options
Hi hackers,
There are lots of subscription options listed on the CREATE
SUBSCRIPTION page [1]https://www.postgresql.org/docs/devel/sql-createsubscription.html.
Although these boolean options are capable of accepting different
values like "1|0", "on|off", "true|false", here they are all described
only using values "true|false".
~
IMO this consistent way of describing the boolean option values ought
to be followed also on the ALTER SUBSCRIPTION page [2]https://www.postgresql.org/docs/devel/sql-altersubscription.html. Specifically,
the ALTER SUBSCRIPTION page has one mention of "SET (failover =
on|off)" which I think should be changed to say "SET (failover =
true|false)"
Now this little change hardly seems important, but actually, it is
motivated by another thread [3]/messages/by-id/8fab8-65d74c80-1-2f28e880@39088166 under development where this ALTER
SUBSCRIPTION "(failover = on|off)" was copied again, thereby making
the consistency between the CREATE SUBSCRIPTION and ALTER SUBSCRIPTION
pages grow further apart, so I think it is best to nip that in the bud
and simply use "true|false" values everywhere.
PSA a patch to make this proposed change.
======
[1]: https://www.postgresql.org/docs/devel/sql-createsubscription.html
[2]: https://www.postgresql.org/docs/devel/sql-altersubscription.html
[3]: /messages/by-id/8fab8-65d74c80-1-2f28e880@39088166
Kind Regards,
Peter Smith.
Fujitsu Australia
Attachments:
v1-0001-Always-use-true-false-instead-of-on-off-for-boole.patchapplication/octet-stream; name=v1-0001-Always-use-true-false-instead-of-on-off-for-boole.patchDownload
From 48b7935e2c14ea94d4d82728e4ceb477c7b23fc8 Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Thu, 16 May 2024 10:15:39 +1000
Subject: [PATCH v1] Always use true|false instead of on|off for boolean
options. This is for consistency with the CREATE SUBSCRIPTION docs page.
---
doc/src/sgml/ref/alter_subscription.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index a78c1c3..476f195 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -69,7 +69,7 @@ ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> RENAME TO <
Commands <command>ALTER SUBSCRIPTION ... REFRESH PUBLICATION</command>,
<command>ALTER SUBSCRIPTION ... {SET|ADD|DROP} PUBLICATION ...</command>
with <literal>refresh</literal> option as <literal>true</literal> and
- <command>ALTER SUBSCRIPTION ... SET (failover = on|off)</command>
+ <command>ALTER SUBSCRIPTION ... SET (failover = true|false)</command>
cannot be executed inside a transaction block.
These commands also cannot be executed when the subscription has
--
1.8.3.1
On Thu, 16 May 2024 at 12:29, Peter Smith <smithpb2250@gmail.com> wrote:
There are lots of subscription options listed on the CREATE
SUBSCRIPTION page [1].Although these boolean options are capable of accepting different
values like "1|0", "on|off", "true|false", here they are all described
only using values "true|false".
If you want to do this, what's the reason to limit it to just this one
page of the docs?
If the following is anything to go by, it doesn't seem we're very
consistent about this over the entire documentation.
doc$ git grep "<literal>on</literal>" | wc -l
122
doc$ git grep "<literal>true</literal>" | wc -l
222
And:
doc$ git grep "<literal>off</literal>" | wc -l
102
doc$ git grep "<literal>false</literal>" | wc -l
162
I think unless we're going to standardise on something then there's
not much point in adjusting individual cases. IMO, there could be an
endless stream of follow-on patches as a result of accepting this.
David
On Thu, May 16, 2024 at 3:11 PM David Rowley <dgrowleyml@gmail.com> wrote:
On Thu, 16 May 2024 at 12:29, Peter Smith <smithpb2250@gmail.com> wrote:
There are lots of subscription options listed on the CREATE
SUBSCRIPTION page [1].Although these boolean options are capable of accepting different
values like "1|0", "on|off", "true|false", here they are all described
only using values "true|false".If you want to do this, what's the reason to limit it to just this one
page of the docs?
Yeah, I had a vested interest in this one place because I've been
reviewing the other thread [1] that was mentioned above. If that other
thread chooses "true|false" then putting "true|false" adjacent to
another "on|off" will look silly. But if that other thread adopts the
existing 'failover=on|off' values then it will diverge even further
from being consistent with the CREATE SUBSCRIPTION page.
Unfortunately, that other thread cannot take it upon itself to make
this change because it has nothing to do with the 'failover' option,
So I saw no choice but to post an independent patch for this.
I checked all the PUBLICATION/SUBSCRIPTION reference pages and this
was the only inconsistent value that I found. But I might be mistaken.
If the following is anything to go by, it doesn't seem we're very
consistent about this over the entire documentation.doc$ git grep "<literal>on</literal>" | wc -l
122doc$ git grep "<literal>true</literal>" | wc -l
222And:
doc$ git grep "<literal>off</literal>" | wc -l
102doc$ git grep "<literal>false</literal>" | wc -l
162
Hmm. I'm not entirely sure if those stats are meaningful because I'm
not saying option values should avoid "on|off" -- the point was I just
suggesting they should be used consistent with where they are
described. For example, the CREATE SUBSCRIPTION page describes every
boolean option value as "true|false", so let's use "true|false" in
every other docs place where those are mentioned. OTOH, other options
on other pages may be described as "on|off" which is fine by me, but
then those should similarly be using "on|off" again wherever they are
mentioned.
I think unless we're going to standardise on something then there's
not much point in adjusting individual cases. IMO, there could be an
endless stream of follow-on patches as a result of accepting this.
Standardisation might be ideal, but certainly, I'm not going to
attempt to make a giant patch that impacts the entire documentation
just for this one small improvement.
It seems a shame if "perfect" becomes the enemy of "good"; How else do
you suggest I can make the ALTER SUBSCRIPTION page better? If this
one-line change is rejected then the most likely outcome is nothing
will ever happen to change it.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
On Thu, 16 May 2024 at 19:05, Peter Smith <smithpb2250@gmail.com> wrote:
On Thu, May 16, 2024 at 3:11 PM David Rowley <dgrowleyml@gmail.com> wrote:
If you want to do this, what's the reason to limit it to just this one
page of the docs?Yeah, I had a vested interest in this one place because I've been
reviewing the other thread [1] that was mentioned above. If that other
thread chooses "true|false" then putting "true|false" adjacent to
another "on|off" will look silly.
OK, looking a bit further I see this option is new to v17. After a
bit more study of the sgml, I agree that it's worth changing this.
I've pushed your patch.
David
Peter Smith <smithpb2250@gmail.com> writes:
Yeah, I had a vested interest in this one place because I've been
reviewing the other thread [1] that was mentioned above. If that other
thread chooses "true|false" then putting "true|false" adjacent to
another "on|off" will look silly. But if that other thread adopts the
existing 'failover=on|off' values then it will diverge even further
from being consistent with the CREATE SUBSCRIPTION page.
It's intentional that we allow more than one spelling of boolean
values. I can see some value in being consistent within nearby
examples, but I don't agree at all that it should be uniform
across all our docs.
regards, tom lane
On Thu, May 16, 2024 at 10:42 PM David Rowley <dgrowleyml@gmail.com> wrote:
On Thu, 16 May 2024 at 19:05, Peter Smith <smithpb2250@gmail.com> wrote:
On Thu, May 16, 2024 at 3:11 PM David Rowley <dgrowleyml@gmail.com> wrote:
If you want to do this, what's the reason to limit it to just this one
page of the docs?Yeah, I had a vested interest in this one place because I've been
reviewing the other thread [1] that was mentioned above. If that other
thread chooses "true|false" then putting "true|false" adjacent to
another "on|off" will look silly.OK, looking a bit further I see this option is new to v17. After a
bit more study of the sgml, I agree that it's worth changing this.I've pushed your patch.
Thanks very much for pushing. It was just a one-time thing -- I won't
go looking for more examples like it.
======
Kind Regards,
Peter Smith.
Fujitsu Australia