Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

Peter Smith <smithpb2250(at)gmail(dot)com>, 21 Tem 2023 Cum, 12:48 tarihinde şunu
yazdı:

> On Fri, Jul 21, 2023 at 5:24 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> >
> > On Fri, Jul 21, 2023 at 12:05 PM Peter Smith <smithpb2250(at)gmail(dot)com>
> wrote:
> > >
> > > On Fri, Jul 21, 2023 at 3:39 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> > > >
>
> > > > The other thing I noticed is that we
> > > > don't seem to be consistent in naming functions in these files. For
> > > > example, shall we make all exposed functions follow camel case (like
> > > > InitializeLogRepWorker) and static functions follow _ style (like
> > > > run_apply_worker) or the other possibility is to use _ style for all
> > > > functions except may be the entry functions like ApplyWorkerMain()? I
> > > > don't know if there is already a pattern but if not then let's form
> it
> > > > now, so that code looks consistent.
> > > >
> > >
> > > +1 for using some consistent rule, but I think this may result in
> > > *many* changes, so it would be safer to itemize all the changes first,
> > > just to make sure everybody is OK with it first before updating
> > > everything.
> > >
> >
> > Fair enough. We can do that as a first patch and then work on the
> > refactoring patch to avoid introducing more inconsistencies or we can
> > do the refactoring patch first but keep all the new function names to
> > follow _ style.
> >
>
> Fixing the naming inconsistency will be more far-reaching than just a
> few functions affected by these "reuse" patches. There are plenty of
> existing functions already inconsistently named in the HEAD code. So
> perhaps this topic should be moved to a separate thread?
>

+1 for moving it to a separate thread. This is not something particularly
introduced by this patch.

Thanks,
--
Melih Mutlu
Microsoft