Re: [PATCH 2/3] perf/aggregate: add --reponame option

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jan 28, 2018, 19:57 UTC
Message-ID: <CAPig+cR77RTTw3uJAwK3XmmvzE7umv3_7M=NkaDEb=uXyNL4FA@mail.gmail.com>
In-Reply-To: <20180128111843.2690-2-chriscool@tuxfamily.org>

On Sun, Jan 28, 2018 at 6:18 AM, Christian Couder <christian.couder@gmail.com> wrote:

Show 32 quoted lines

> This makes it easier to use the aggregate script
> on the command line when one wants to get the
> "environment" fields set in the codespeed output.
>
> Previously setting GIT_REPO_NAME was needed
> for this purpose.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
> @@ -209,15 +218,17 @@ sub print_codespeed_results {
> -       my $environment;
> -       if (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") {
> -               $environment = $ENV{GIT_PERF_REPO_NAME};
> -       } elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") {
> -               $environment = $ENV{GIT_TEST_INSTALLED};
> -               $environment =~ s|/bin-wrappers$||;
> -       } else {
> -               $environment = `uname -r`;
> -               chomp $environment;
> +       my $environment = $reponame;
> +       if (! $environment) {
> +               if (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") {
> +                       $environment = $ENV{GIT_PERF_REPO_NAME};
> +               } elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") {
> +                       $environment = $ENV{GIT_TEST_INSTALLED};
> +                       $environment =~ s|/bin-wrappers$||;
> +               } else {
> +                       $environment = `uname -r`;
> +                       chomp $environment;
> +               }
>         }

Not a big deal, but the extra indentation (and noisy diff) could be avoided like this:

    my $environment;
    if ($reponame) {
        $environment = $reponame;
    } else if (exists ...) {
        ...as before....
    }