public inbox for [email protected]  
help / color / mirror / Atom feed
From: Michael Paquier <[email protected]>
To: Robert Haas <[email protected]>
Cc: Amul Sul <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Add system identifier to backup manifest
Date: Mon, 19 Feb 2024 07:52:05 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+TgmoYrs8A2j-4KhkjtKp6MwJRwOkXHdXVg4DE6nDOMgDBxMA@mail.gmail.com>
References: <CAAJ_b96uU1aSvuzTt4AO+y18Z+o=_s8Zxbo2Dy4Wn=5vRGKPKw@mail.gmail.com>
	<CA+Tgmoa9HVHVP0yishY5h81CtzT57s4ttsULCKeN-i10bTUZ8A@mail.gmail.com>
	<CAAJ_b9417seAdhwhMVr=PnQ+MPx7uAeS5FAw2qMCXF5MrE-7wQ@mail.gmail.com>
	<CA+Tgmoa78r+U-X2EoGrvNDuKur5MhMtvZbm01-c1ziRgSq9MEA@mail.gmail.com>
	<CAAJ_b97pp0=xXwW=VKWTxKDF2r7uYXYxZnTggF6dRHtnjwi+WA@mail.gmail.com>
	<CA+Tgmoa8okr1YCFcCOGgvOTASiXsJgx-oqrCS41Wir=oC8c-mA@mail.gmail.com>
	<CAAJ_b95tfUo9CyCX-=0V1L=VNzzvcf4dGLT6ae7=kz2ELy2Edg@mail.gmail.com>
	<[email protected]>
	<CAAJ_b961KftvFLJYvJ1-CWk13dLVcb0+o=q0BRd3n8NJ-ceWBQ@mail.gmail.com>
	<CA+TgmoYrs8A2j-4KhkjtKp6MwJRwOkXHdXVg4DE6nDOMgDBxMA@mail.gmail.com>

On Thu, Feb 15, 2024 at 05:41:46PM +0530, Robert Haas wrote:
> On Thu, Feb 15, 2024 at 3:05 PM Amul Sul <[email protected]> wrote:
> > Kindly have a look at the attached version.
> 
> IMHO, 0001 looks fine, except probably the comment could be phrased a
> bit more nicely.

And the new option should be documented at the top of the init()
routine for perldoc.

> That can be left for whoever commits this to
> wordsmith. Michael, what are your plans?

Not much, so feel free to not wait for me.  I've just read through the
patch because I like the idea/feature :)

> 0002 seems like a reasonable approach, but there's a hunk in the wrong
> patch: 0004 modifies pg_combinebackup's check_control_files to use
> get_dir_controlfile() rather than git_controlfile(), but it looks to
> me like that should be part of 0002.

I'm slightly concerned about 0002 that silently changes the meaning of
get_controlfile().  That would impact extension code without people
knowing about it when compiling, just when they run their stuff under
17~.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
  download

view thread (7+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Add system identifier to backup manifest
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox