Re: refactoring basebackup.c

Hello

I found that in 0001 you propose to rename few options. Probably we could rename another option for clarify? I think FAST (it's about some bw limits?) and WAIT (wait for what? checkpoint?) option names are confusing.
Could we replace FAST with "CHECKPOINT [fast|spread]" and WAIT to WAIT_WAL_ARCHIVED? I think such names would be more descriptive.

- if (PQserverVersion(conn) >= 100000)
- /* pg_recvlogical doesn't use an exported snapshot, so suppress */
- appendPQExpBufferStr(query, " NOEXPORT_SNAPSHOT");
+ /* pg_recvlogical doesn't use an exported snapshot, so suppress */
+ if (use_new_option_syntax)
+ AppendStringCommandOption(query, use_new_option_syntax,
+ "SNAPSHOT", "nothing");
+ else
+ AppendPlainCommandOption(query, use_new_option_syntax,
+ "NOEXPORT_SNAPSHOT");

In 0002, it looks like condition for 9.x releases was lost?

Also my gcc version 8.3.0 is not happy with v5-0007-Support-base-backup-targets.patch and produces:

basebackup.c: In function ‘parse_basebackup_options’:
basebackup.c:970:7: error: ‘target_str’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
errmsg("target '%s' does not accept a target detail",
^~~~~~

regards, Sergei