Code review comment for lp:~percona-toolkit-dev/percona-toolkit/OptionParser-remove-optional_value

In Pingback.pm:

sub version_check {
   my $args = pop @_;
   my (@instances) = @_;

That should be our standard %args.

      print STDERR '--version-check is disabled by the PERCONA_VERSION_CHECK ',
                   "environment variable.\n\n";

I think those should just be warn "..." (we tend not to use print STDERR for whatever reason).

      $args->{protocol} ||= 'https';
      my @protocols = $args->{protocol} ne 'https' && $args->{protocol} ne 'http'
                    ? qw(https http)
                    : $args->{protocol};

Let's do this earlier, before the eval, and check for 'off'. If 'off', then return. This will need to happen in the future when we switch the default to 'auto'.

Also, make the ?: more clear because right now it reads like "if not https and not http, then use https and http"--huh? (because 'auto', but that's not clear).

Also, --version-check '' on the cmd line isn't handled, so we need to check the --version-check value before $o->usage_or_errors(). To do this, make a MAGIC_version_check_protocols lists and use that so the docs become the actual values we check for (see MAGIC_history_cols in pqd for an example).

While v-c is a manual thing, let's increase,

   $ua ||= HTTPMicro->new( timeout => 2 );

to 5 since the user, knowing what v-c does, should be a little more lenient (and eventually they'll get either "No advice" or a warning or something, so they'll know the delay was due to v-c working).