Re: PGdoc: add missing ID attribute to create_subscription.sgml
От | Peter Smith |
---|---|
Тема | Re: PGdoc: add missing ID attribute to create_subscription.sgml |
Дата | |
Msg-id | CAHut+Pu+-OocYYhW9E0gxxqgfUb1yJ8jVQ4AZ0v-ud00s7TxEA@mail.gmail.com обсуждение исходный текст |
Ответ на | PGdoc: add missing ID attribute to create_subscription.sgml ("Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>) |
Ответы |
RE: PGdoc: add missing ID attribute to create_subscription.sgml
("Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>)
|
Список | pgsql-hackers |
Firstly, +1 for this patch. Directly jumping to the subscription options makes it much easier to navigate in the documentation instead of scrolling up and done in CREATE SUBSCRIPTION page looking for each parameter. Already (just experimenting with this patch) it is noticeably better. ~~ Anyway, here are my review comments for patch 0001 ====== General 1. It will be better if all the references follow a consistent pattern: Rule 1 - IMO it is quite important/necessary for these option name “XXX” (see below) to be rendered using <literal> markup rather than just plain text font. Unfortunately, I don't know how to do that using xref labels. If you can figure out some way to do it then great, otherwise I feel it is better just remove all those xreflabels and instead create the links like this: <link linkend="sql-createsubscription-with-XXX"><literal>XXX</literal></link> option Rule 2 – Try to keep consistent phrasing like "XXX option" or "XXX parameter" (whatever is appropriate for the neighbouring text) ~~~ 2. I think you can extend this patch similarly to add IDs for the WITH parameters of CREATE PUBLICATION. For example, I saw a couple of places where referencing the 'publish' parameter might be useful. ====== Commit message 3. Currently, there is nothing. ====== doc/src/sgml/config.sgml 4. (Section 20.17 Developer Options -- logical_replication_mode) - <literal>streaming</literal> option (see optional parameters set by - <link linkend="sql-createsubscription"><command>CREATE SUBSCRIPTION</command></link>) + <xref linkend="sql-createsubscription-with-streaming"/> option + (see optional parameters set by <link linkend="sql-createsubscription"><command>CREATE SUBSCRIPTION</command></link>) Since we now have a direct link to the option, I think the rest of that sentence can now be a bit simpler. YMMV. SUGGESTION (per my general comment about links/fonts) ... if the <link linkend="sql-createsubscription-with-streaming"><literal>streaming</literal></link> option of <link linkend="sql-createsubscription"><command>CREATE SUBSCRIPTION</command></link> is enabled, otherwise, serialize each change. ====== doc/src/sgml/logical-replication. 5. (Section 31.2 Subscription) - <literal>streaming</literal> option (see optional parameters set by - <link linkend="sql-createsubscription"><command>CREATE SUBSCRIPTION</command></link>) + <xref linkend="sql-createsubscription-with-streaming"/> option + (see optional parameters set by <link linkend="sql-createsubscription"><command>CREATE SUBSCRIPTION</command></link>) For consistency with everything else, I think only the word “binary should be the link. SUGGESTION See the <link linkend="sql-createsubscription-with-binary"><literal>binary</literal></link> option ... ~~~ 6. (Section 31.2.3 Examples) - restrictive. See the <link linkend="sql-createsubscription-binary"><literal>binary</literal> + restrictive. See the <link linkend="sql-createsubscription-with-binary"><literal>binary</literal> SUGGESTION (per my general comment about links/fonts, and also added word "option") <link linkend="sql-createsubscription-with-slot-name"><literal>slot_name</literal></link> option. ~~~ 7. (Section 31.5 Conflicts) - subscription can be used with the <literal>disable_on_error</literal> option. - Then, you can use <function>pg_replication_origin_advance()</function> function - with the <parameter>node_name</parameter> (i.e., <literal>pg_16395</literal>) + subscription can be used with the <xref linkend="sql-createsubscription-with-disable-on-error"/> + option. Then, you can use <function>pg_replication_origin_advance()</function> + function with the <parameter>node_name</parameter> (i.e., <literal>pg_16395</literal>) SUGGESTION (per my general comment about links/fonts) <link linkend="sql-createsubscription-with-disable-on-error"><literal>disable_on_error</literal></link> ====== doc/src/sgml/ref/alter_subscription.sgml 8. (Description) - <literal>two_phase</literal> commit enabled, - unless <literal>copy_data</literal> is <literal>false</literal>. + <link linkend="sql-createsubscription-with-two-phase"> commit enabled</link>, + unless <xref linkend="sql-createsubscription-with-copy-data"/> is <literal>false</literal>. I think the "two_phase" was rendering wrongly because there was a mixup of link/xref. Suggest fix it like below: SUGGESTION (per my general comment about links/fonts) <link linkend="sql-createsubscription-with-two-phase"><literal>two_phase</literal></link> commit enabled, unless <link linkend="sql-createsubscription-with-copy-data"><literal>copy_data</literal></link> is <literal>false</literal>. ~~~ 9. (copy_data) - <literal>origin</literal> parameter. + <xref linkend="sql-createsubscription-with-origin"/> parameter. SUGGESTION (per my general comment about links/fonts) <link linkend="sql-createsubscription-with-origin"><literal>origin</literal></link> parameter. ~ 10. <para> - See the <link linkend="sql-createsubscription-binary"><literal>binary</literal> + See the <link linkend="sql-createsubscription-with-binary"><literal>binary</literal> Everything nearby was called a "parameter" so I recommend to change "binary option" to "binary parameter" here too and move that word outside the link. SUGGESTION (per my general comment about links/fonts) See the <link linkend="sql-createsubscription-with-binary"><literal>binary</literal></link> parameter of ... ~~~ 11 (SET) - are <literal>slot_name</literal>, - <literal>synchronous_commit</literal>, - <literal>binary</literal>, <literal>streaming</literal>, - <literal>disable_on_error</literal>, and + are <xref linkend="sql-createsubscription-with-slot-name"/>, + <xref linkend="sql-createsubscription-with-synchronous-commit"/>, + <literal>binary</literal>, <xref linkend="sql-createsubscription-with-streaming"/>, + <xref linkend="sql-createsubscription-with-disable-on-error"/>, and Modify so all the fonts are <literal>. Also, the binary link and origin links were added. I know you said you chose to do that because they are already linked previously on this page, but in practice, it looked strange when rendered where only those ones were missing as links from this long list. SUGGESTION (per my general comment about links/fonts) The parameters that can be altered are <link linkend="sql-createsubscription-with-slot-name"><literal>slot_name</literal></link>, <link linkend="sql-createsubscription-with-synchronous-commit"><literal>synchronous_commit</literal></link>, <link linkend="sql-createsubscription-with-binary"><literal>binary</literal></link>, <link linkend="sql-createsubscription-with-streaming"><literal>streaming</literal></link>, <link linkend="sql-createsubscription-with-disable-on-error"><literal>disable_on_error</literal></link>, and <link linkend="sql-createsubscription-with-origin"><literal>origin</literal></link>. ====== doc/src/sgml/ref/create_subscription.sgml 12. I think all those xreflabels can be removed. As per my general comment, the references to the WITH option should use a <literal> font for the option name, but then I was unable to get that working using xreflabels. So AFAIK those xreflabels are unused (unless they have some other purpose that I don't know about). ~~~ 13. Sometimes the WITH parameters reference to each other on this page. I wasn’t sure if we should cross-reference within the same page. What do you think? It might be useful, or OTOH it might be overkill to have too many links. e.g. connect refers to -- create_slot, enabled, copy_data e.g. a lot_name refers to -- create_slot, enabled e.g. binary refers to -- copy_data e.g. copy_data refers to -- origin e.g. origin refers to -- copy_data ====== doc/src/sgml/ref/pg_dump.sgml 14. (Section II. PG client applications -- pg_dump) - <literal>two_phase</literal> option will be automatically enabled by the - subscriber if the subscription had been originally created with - <literal>two_phase = true</literal> option. + <xref linkend="sql-createsubscription-with-two-phase"/> option will be + automatically enabled by the subscriber if the subscription had been + originally created with <literal>two_phase = true</literal> option. SUGGESTION (per my general comment about links/fonts) <link linkend="sql-createsubscription-with-two-phase"><literal>two_phase</literal></link> option ------ Kind Regards, Peter Smith. Fujitsu Australia
В списке pgsql-hackers по дате отправления:
Следующее
От: torikoshiaДата:
Сообщение: Re: POC PATCH: copy from ... exceptions to: (was Re: VLDB Features)