Обсуждение: Docs: Always use true|false instead of sometimes on|off for the subscription options
Docs: Always use true|false instead of sometimes on|off for the subscription options
От
Peter Smith
Дата:
Hi hackers, 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". ~ IMO this consistent way of describing the boolean option values ought to be followed also on the ALTER SUBSCRIPTION page [2]. 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] 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] https://www.postgresql.org/message-id/flat/8fab8-65d74c80-1-2f28e880%4039088166 Kind Regards, Peter Smith. Fujitsu Australia
Вложения
Re: Docs: Always use true|false instead of sometimes on|off for the subscription options
От
David Rowley
Дата:
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 > 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 > 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
Re: Docs: Always use true|false instead of sometimes on|off for the subscription options
От
David Rowley
Дата:
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