From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <ci-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id D2AFB43252;
	Tue, 31 Oct 2023 15:45:24 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id CC874402D7;
	Tue, 31 Oct 2023 15:45:24 +0100 (CET)
Received: from us-smtp-delivery-124.mimecast.com
 (us-smtp-delivery-124.mimecast.com [170.10.129.124])
 by mails.dpdk.org (Postfix) with ESMTP id 3C33D40284
 for <ci@dpdk.org>; Tue, 31 Oct 2023 15:45:24 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
 s=mimecast20190719; t=1698763523;
 h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
 to:to:cc:cc:mime-version:mime-version:content-type:content-type:
 content-transfer-encoding:content-transfer-encoding:
 in-reply-to:in-reply-to:references:references;
 bh=BtExJEH/FRucHZzfA0KVtfKnCcQHLg89hIRDmiNCbB0=;
 b=QmZ6b4ks7yItXZh1dvRvMPvJXzbZa3LUZTL9CROdoVRioV1G5u5fLblU2mvmHXea9K/fVN
 mGHitE0apRX2Qc8pjANrP2zXn+pNMIsS/9nSUJp5IHPbieKJDO0Wd3TDrlpjdcpwPo0hJe
 pCKjrEAkahO3AnCv2RkfuQ54xQ1LaGA=
Received: from mail-ej1-f71.google.com (mail-ej1-f71.google.com
 [209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS
 (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id
 us-mta-433-dpyw22mQMKaeAPKSqMmGiw-1; Tue, 31 Oct 2023 10:45:22 -0400
X-MC-Unique: dpyw22mQMKaeAPKSqMmGiw-1
Received: by mail-ej1-f71.google.com with SMTP id
 a640c23a62f3a-9d30a6a67abso170165066b.2
 for <ci@dpdk.org>; Tue, 31 Oct 2023 07:45:22 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20230601; t=1698763521; x=1699368321;
 h=content-transfer-encoding:cc:to:subject:message-id:date:from
 :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc
 :subject:date:message-id:reply-to;
 bh=BtExJEH/FRucHZzfA0KVtfKnCcQHLg89hIRDmiNCbB0=;
 b=fB/pTuZQSmdtpLcfjGWETyt/Hcn8ExI8ifHxmmxue+QqkiZXCwv9CwpG88ObdCYy5y
 41B1p0Nmly0YP25Tw1LZ2bFm9gTEp5ObnpmtWjbly1QZhpAgmzoJc54Zd5v8gSBpL3Ug
 IOVpHb4XbbVZSrp8J365+fhWsmmfjNTUDE/Ipi+zP5jezV2a7u1HVwxU9UatjMK6aDQ2
 UVdcdu5qmpqQael/BV9axFoDdWOYniiVxYRcPlEGroDg8dXeCA/w8TubCC1R4PkBReXm
 pcGUBlhkq4xfUjbsnnY0aTTNvoVPFrEZtN12j5AGWsjfAw7fR0xqvfAfUH7vEAHVFovf
 mSMw==
X-Gm-Message-State: AOJu0YzbTZEyNX8EU92bg03IxqIH6YeVTIjwEUN+w7jEaAvLYLmQ8xYE
 kKpEGktbIxBmi52X1SQUQQTr8e+hbxDRtX4omu/xnNa8dnsf4Rgao7ci+6HK4sjxtyOb4uoVCVY
 eLSRUN9oYwICUfHnfWg==
X-Received: by 2002:a17:907:7da7:b0:9bf:7a4d:5913 with SMTP id
 oz39-20020a1709077da700b009bf7a4d5913mr12072300ejc.2.1698763521244; 
 Tue, 31 Oct 2023 07:45:21 -0700 (PDT)
X-Google-Smtp-Source: AGHT+IF3SKKXARRXIWDC7z+CbpUa79uSgBYaXilpFG96X/RyrMgir66OPTuMxfJcyuNnxNPxszDKetqilIeEAKt8dig=
X-Received: by 2002:a17:907:7da7:b0:9bf:7a4d:5913 with SMTP id
 oz39-20020a1709077da700b009bf7a4d5913mr12072284ejc.2.1698763520971; Tue, 31
 Oct 2023 07:45:20 -0700 (PDT)
MIME-Version: 1.0
References: <20231027130609.3035396-1-aconole@redhat.com>
In-Reply-To: <20231027130609.3035396-1-aconole@redhat.com>
From: Michael Santana <msantana@redhat.com>
Date: Tue, 31 Oct 2023 10:45:09 -0400
Message-ID: <CABVNPRpONHm4r3SzxDjRXT+4nOWRwOmBAoS34U3b5YuxSc8mBQ@mail.gmail.com>
Subject: Re: [RFC 1/2] pw_mon: improve command line options
To: Aaron Conole <aconole@redhat.com>
Cc: Dumitru Ceara <dceara@redhat.com>, David Marchand <dmarchan@redhat.com>, 
 Ilya Maximets <imaximet@redhat.com>, ci@dpdk.org
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-BeenThere: ci@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK CI discussions <ci.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/ci>,
 <mailto:ci-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/ci/>
List-Post: <mailto:ci@dpdk.org>
List-Help: <mailto:ci-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/ci>,
 <mailto:ci-request@dpdk.org?subject=subscribe>
Errors-To: ci-bounces@dpdk.org

On Fri, Oct 27, 2023 at 9:06=E2=80=AFAM Aaron Conole <aconole@redhat.com> w=
rote:
>
> In the future, we'll use this to add support for passing opts into some p=
arts
> of pw_mon.
>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  pw_mon | 53 +++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 39 insertions(+), 14 deletions(-)
>
> diff --git a/pw_mon b/pw_mon
> index 28feb8b..01bdd25 100755
> --- a/pw_mon
> +++ b/pw_mon
> @@ -21,34 +21,59 @@
>
>  [ -f "${HOME}/.pwmon-rc" ] && source "${HOME}/.pwmon-rc"
>
> -if [ "$1" !=3D "" ]; then
> -    pw_project=3D"$1"
> -    shift
> +if [ "$1" !=3D ""  ]; then
nitpick: there is an extra white space between "" and ]
> +    if ! echo "$1" | grep -E ^-- >/dev/null 2>&1; then
I am trying to understand what you are trying to accomplish with these
if statements and the while loop. Couldnt you just move everything
into the while loop so you dont have to repeat the checks? I mean, you
will probably still have to use an OR || to check both conditions,
with and without the "--" at the begining. Maybe you could use some
sort of regex to make the "--" optional and then maybe you could
combine both checks into a single check and make it much easier

But just to double check, you are trying to parse both
"--pw-instance=3Dmyinstance" and "pw-instance=3Dmyinstance". Correct?
That's the whole goal of this patch?

The lack of strings around ^-- make me really unseasy, this also
applies to the sed line. Also, with grep you can tell it to not dump
any output/error. see -s and -q options. These apply to the other grep
commands in the script as well

> +        pw_project=3D"$1"
> +        shift
> +    fi
>  fi
>
>  if [ "$1" !=3D "" ]; then
> -    pw_instance=3D"$1"
> -    shift
> -fi
> -
> -if [ "X$pw_instance" =3D=3D "X" -o "X$pw_project" =3D=3D "X" ]; then
> -   echo "ERROR: Patchwork instance and project are unset."
> -   echo "Please setup ${HOME}/.pwmon-rc and set pw_project "
> -   echo "(or pass it as an argument)."
> -   echo "Also either setup pw_instance or pass it as an argument."
> -   exit 1
> +    if ! echo "$1" | grep -E ^-- >/dev/null 2>&1; then
> +        pw_instance=3D"$1"
> +        shift
> +    fi
>  fi
>
>  userpw=3D""
>
>  if [ "$1" !=3D "" ]; then
> -    pw_credential=3D"$1"
> +    if ! echo "$1" | grep -E ^-- >/dev/null 2>&1; then
why is this not inside the while loop? the use of -- forbidden for
credentials via the cli?
> +        pw_credential=3D"$1"
> +        shift
> +    fi
>  fi
>
> +
> +while [ "$1" !=3D "" ]; do
Im guessing the whole point of this patch was to add this while loop
> +    if echo "$1" | grep -E ^--pw-project=3D >/dev/null 2>&1; then
> +        pw_project=3D$(echo "$1" | sed s/--pw-project=3D//)
> +        shift
> +    elif echo "$1" | grep -E ^--pw-instance=3D >/dev/null 2>&1; then
> +        pw_instance=3D$(echo "$1" | sed s/--pw-instance=3D//)
> +        shift
> +    elif echo "$1" | grep -E ^--help >/dev/null 2>&1; then
> +        echo "pw_mon script"
> +        echo "$0 [proj|--pw-project=3D<proj>] [instance|--pw-instance=3D=
<inst url] opts.."
> +        echo "Options:"
> +        echo "    --add-filter-recheck=3Dfilter  Adds a filter to flag t=
hat a recheck needs to be done"
> +        echo ""
> +        exit 0
> +    fi
> +done
> +
>  if [ "X$pw_credential" !=3D "X" ]; then
>     userpw=3D"-u \"${pw_credential}\""
>  fi
>
> +if [ "X$pw_instance" =3D=3D "X" -o "X$pw_project" =3D=3D "X" ]; then
> +   echo "ERROR: Patchwork instance and project are unset."
> +   echo "Please setup ${HOME}/.pwmon-rc and set pw_project "
> +   echo "(or pass it as an argument)."
> +   echo "Also either setup pw_instance or pass it as an argument."
> +   exit 1
> +fi
> +
>  source $(dirname $0)/series_db_lib.sh
>
>  function emit_series() {
> --
> 2.41.0
>