> Here are some comments for patch v2. > > ====== > > 1. There are whitespace problems > > [postgres@CentOS7-x64 oss_postgres_misc]$ git apply > ../patches_misc/v2-0001-Add-publisher-and-subscriber-to-glossary-document.patch > ../patches_misc/v2-0001-Add-publisher-and-subscriber-to-glossary-document.patch:43: > trailing whitespace. > A node where publication is defined for > ../patches_misc/v2-0001-Add-publisher-and-subscriber-to-glossary-document.patch:45: > trailing whitespace. > It replicates a set of changes from a table or a group of tables in > ../patches_misc/v2-0001-Add-publisher-and-subscriber-to-glossary-document.patch:66: > trailing whitespace. > A node where subscription is defined for > ../patches_misc/v2-0001-Add-publisher-and-subscriber-to-glossary-document.patch:67: > trailing whitespace. > <glossterm linkend="glossary-replication">logical > replication</glossterm>. > ../patches_misc/v2-0001-Add-publisher-and-subscriber-to-glossary-document.patch:68: > trailing whitespace. > It subscribes to one or more publications on a publisher node and pulls > warning: squelched 2 whitespace errors > warning: 7 lines add whitespace errors. > > ~~~ > 2. Publisher node > > + <glossentry id="glossary-publisher"> > + <glossterm>Publisher node</glossterm> > + <glossdef> > + <para> > + A node where publication is defined for > + <glossterm linkend="glossary-replication">logical > replication</glossterm>. > + It replicates a set of changes from a table or a group of tables in > + publication to the subscriber node. > + </para> > + <para> > + For more information, see > + <xref linkend="logical-replication-publication"/>. > + </para> > + </glossdef> > + </glossentry> > + > > > 2a. > I felt the term here should be "Publication node". > > Indeed, there should also be additional glossary terms like > "Publisher" (i.e. it is the same as "Publication node") and > "Subscriber" (i.e. it is the same as "Subscription node"). These > definitions will then be consistent with node descriptions already in > sections 30.1 and 30.2 > > > ~ > > 2b. > "node" should include a link to the glossary term. Same for any other > terms mentioned > > ~ > > 2c. > /A node where publication is defined for/A node where a publication is > defined for/ > > ~ > > 2d. > "It replicates" is misleading because it is the PUBLICATION doing the > replicating, not the node. > > IMO it will be better to include 2 more glossary terms "Publication" > and "Subscription" where you can say this kind of information. Then > the link "logical-replication-publication" also belongs under the > "Publication" term. > > > ~~~ > > 3. > + <glossentry id="glossary-subscriber"> > + <glossterm>Subscriber node</glossterm> > + <glossdef> > + <para> > + A node where subscription is defined for > + <glossterm linkend="glossary-replication">logical > replication</glossterm>. > + It subscribes to one or more publications on a publisher node and pulls > + a set of changes from a table or a group of tables in publications it > + subscribes to. > + </para> > + <para> > + For more information, see > + <xref linkend="logical-replication-subscription"/>. > + </para> > + </glossdef> > + </glossentry> > > All comments are similar to those above. > > ====== > > In summary, IMO it should be a bit more like below: > > SUGGESTION (include all the necessary links etc) > > Publisher > See "Publication node" > > Publication > A publication replicates the changes of one or more tables to a > subscription. For more information, see Section 30.1 > > Publication node > A node where a publication is defined for logical replication. > > Subscriber > See "Subscription node" > > Subscription > A subscription receives the changes of one or more tables from the > publications it subscribes to. For more information, see Section 30.2 > > Subscription node > A node where a subscription is defined for logical replication.
I have addressed the comments and added an updated patch. Thanks and Regards, Shlok Kyal
v3-0001-Add-publisher-and-subscriber-to-glossary-document.patch
Description: Binary data