On Tue, Mar 18, 2025 at 4:25 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Mon, 17 Mar 2025 at 16:51, Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
> >
> > On Mon, Mar 17, 2025 at 4:02 PM vignesh C <vignesh21@gmail.com> wrote:
> > >
> > > On Mon, 17 Mar 2025 at 11:28, Shubham Khanna
> > > <khannashubham1197@gmail.com> wrote:
> > > >
> > > > On Fri, Mar 14, 2025 at 5:43 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> > > >
> > > > Fixed.
> > > >
> > > > The attached patch contains the suggested changes.
> > >
> > > I feel like we're trying to address two separate tasks in this thread:
> > > a) Enhancing pg_createsubscriber to automatically retrieve databases
> > > when none is provided. b) Refactoring all pg_createsubscriber tests.
> > >
> > > I suggest we keep this thread focused solely on retrieving all
> > > databases and start a new thread for test refactoring. This will allow
> > > us to tackle each task separately and ensure a cleaner commit.
> >
> > I was expecting that the argument validation tests will go in one test
> > - existing as well as the ones for --all. But that's not how the patch
> > is splitting them. It has split only the existing test. I am fine if
> > we add a new test for --all option as the [patch does and leave the
> > existing test as is. Cramming everything in one test makes it
> > unmaintainable, though.
>
> For this patch, let's add an additional test case to
> 040_pg_createsubscriber.pl and aim to commit it soon, as the rest of
> the changes look good. I agree that the test split you suggested is
> necessary, but let's handle that in a separate thread.
I am fine with it. I am just worried about the resultant test being
unreadable and patch hard to review. That's how the first patch was
written.
--
Best Wishes,
Ashutosh Bapat
