Thread: logical/relation.c header description

logical/relation.c header description

From
Amit Langote
Date:
Hi,

$subect needs fixing, because right now it says just:

 /*-------------------------------------------------------------------------
  * relation.c
  *    PostgreSQL logical replication

Attached find a patch to expand that a little bit.

-- 
Amit Langote
EnterpriseDB: http://www.enterprisedb.com

Attachment

Re: logical/relation.c header description

From
Amit Kapila
Date:
On Thu, Sep 17, 2020 at 3:22 PM Amit Langote <amitlangote09@gmail.com> wrote:
>

 /*-------------------------------------------------------------------------
  * relation.c
- *    PostgreSQL logical replication
+ *    PostgreSQL logical replication relation mapping routines
  *
..
 * NOTES
 *   This file contains helper functions for logical replication relation
 *   mapping cache.

The new header title and NOTES section say the almost same thing. How
about changing the title as "PostgreSQL logical replication relation
mapping cache" and remove the NOTES section.

-- 
With Regards,
Amit Kapila.



Re: logical/relation.c header description

From
Amit Langote
Date:
Thanks for taking a look.

On Thu, Sep 17, 2020 at 9:20 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Thu, Sep 17, 2020 at 3:22 PM Amit Langote <amitlangote09@gmail.com> wrote:
> >
>
>  /*-------------------------------------------------------------------------
>   * relation.c
> - *    PostgreSQL logical replication
> + *    PostgreSQL logical replication relation mapping routines
>   *
> ..
>  * NOTES
>  *   This file contains helper functions for logical replication relation
>  *   mapping cache.
>
> The new header title and NOTES section say the almost same thing. How
> about changing the title as "PostgreSQL logical replication relation
> mapping cache" and remove the NOTES section.

That makes sense.

Actually, I did consider expanding that NOTE to mention what the
module does but there are not actually that many interesting things in
there.  If you would like to take a look at the text I had come up
with, please check the attached.

-- 
Amit Langote
EnterpriseDB: http://www.enterprisedb.com

Attachment

Re: logical/relation.c header description

From
Amit Kapila
Date:
On Thu, Sep 17, 2020 at 6:46 PM Amit Langote <amitlangote09@gmail.com> wrote:
>
> Thanks for taking a look.
>
> On Thu, Sep 17, 2020 at 9:20 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Thu, Sep 17, 2020 at 3:22 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > >
> >
> >  /*-------------------------------------------------------------------------
> >   * relation.c
> > - *    PostgreSQL logical replication
> > + *    PostgreSQL logical replication relation mapping routines
> >   *
> > ..
> >  * NOTES
> >  *   This file contains helper functions for logical replication relation
> >  *   mapping cache.
> >
> > The new header title and NOTES section say the almost same thing. How
> > about changing the title as "PostgreSQL logical replication relation
> > mapping cache" and remove the NOTES section.
>
> That makes sense.
>
> Actually, I did consider expanding that NOTE to mention what the
> module does but there are not actually that many interesting things in
> there.  If you would like to take a look at the text I had come up
> with, please check the attached.
>

I am not very excited to add information about structures used in the
file especially when we have explained 'LogicalRepPartMapEntry' a few
lines after. I think we can leave explaining structures but otherwise,
it looks good. See attached.

-- 
With Regards,
Amit Kapila.

Attachment

Re: logical/relation.c header description

From
Amit Langote
Date:
On Fri, Sep 18, 2020 at 12:30 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Thu, Sep 17, 2020 at 6:46 PM Amit Langote <amitlangote09@gmail.com> wrote:
> >
> > Thanks for taking a look.
> >
> > On Thu, Sep 17, 2020 at 9:20 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > On Thu, Sep 17, 2020 at 3:22 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > > >
> > >
> > >  /*-------------------------------------------------------------------------
> > >   * relation.c
> > > - *    PostgreSQL logical replication
> > > + *    PostgreSQL logical replication relation mapping routines
> > >   *
> > > ..
> > >  * NOTES
> > >  *   This file contains helper functions for logical replication relation
> > >  *   mapping cache.
> > >
> > > The new header title and NOTES section say the almost same thing. How
> > > about changing the title as "PostgreSQL logical replication relation
> > > mapping cache" and remove the NOTES section.
> >
> > That makes sense.
> >
> > Actually, I did consider expanding that NOTE to mention what the
> > module does but there are not actually that many interesting things in
> > there.  If you would like to take a look at the text I had come up
> > with, please check the attached.
> >
> I am not very excited to add information about structures used in the
> file especially when we have explained 'LogicalRepPartMapEntry' a few
> lines after. I think we can leave explaining structures but otherwise,
> it looks good. See attached.

Works for me, thanks.

-- 
Amit Langote
EnterpriseDB: http://www.enterprisedb.com



Re: logical/relation.c header description

From
Amit Kapila
Date:
On Fri, Sep 18, 2020 at 9:07 AM Amit Langote <amitlangote09@gmail.com> wrote:
>
> On Fri, Sep 18, 2020 at 12:30 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > I am not very excited to add information about structures used in the
> > file especially when we have explained 'LogicalRepPartMapEntry' a few
> > lines after. I think we can leave explaining structures but otherwise,
> > it looks good. See attached.
>
> Works for me, thanks.
>

Pushed, thanks!

-- 
With Regards,
Amit Kapila.