From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id EDEBC45460; Fri, 14 Jun 2024 22:29:38 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7D27240674; Fri, 14 Jun 2024 22:29:38 +0200 (CEST) Received: from mail-pj1-f50.google.com (mail-pj1-f50.google.com [209.85.216.50]) by mails.dpdk.org (Postfix) with ESMTP id 933034060B for ; Fri, 14 Jun 2024 22:29:36 +0200 (CEST) Received: by mail-pj1-f50.google.com with SMTP id 98e67ed59e1d1-2bda9105902so2152628a91.0 for ; Fri, 14 Jun 2024 13:29:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1718396976; x=1719001776; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=MVZzpYbgO/KuctE92/Q7sitB5ilNHTO0pu+D5MCHhlU=; b=E6ShiYMuuOOrDbAYfnwHMjQ9pNSx/cLNDeE60PL1DtFMRF0Nvf8ttkV7cI8huioSOX V20njP6qxGYq08TtZZELA/itL5gJvoAPX7BMTVkCFDqQf3MKSLtzsalU+7y4SYgKX9Vq ymJW+39pC76W7S0XXiOiGF7dqhvcZETLaqg98= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718396976; x=1719001776; 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=MVZzpYbgO/KuctE92/Q7sitB5ilNHTO0pu+D5MCHhlU=; b=iJKr3FR5yYInD9lZnCD3H5wA77dL1+OmZOOPDQgA++O3cxMau/xKRy1LI8EO04lg+f cfA8gskAjZJ8QiWjUYGlN+WE3mi8sotL8EdxcpzDdeKFqmcU6kx3QrsOpw/4u93e9lmP y0KSK0ddZNyRefNjPnDqzsYbsvNPyzXtxbphQk2wHptTQ6HNxWod9KrCHWV6RrRvbZWP gdSA3QxJsEFVVTHwjkU87+XAt6YObUjIXWqJrGza6v7jvcXEflPWD/8hKG6xwD5V7ljw D3chnRs3pG3ukrF3fJHDO6FLnCYuZSyLNvpMuGK2tq96RViq0+/f6oQXOKnoxj8SwHjF YWfg== X-Forwarded-Encrypted: i=1; AJvYcCWq0QfFldwLgORXw31cc7iKMQi2J66HmsQaRNNpKYQ2lmtYfpBP4XnnsB7eUdXcxL8rZlwoslcEHaZYaL4= X-Gm-Message-State: AOJu0YyhFuc3A9IoGrZTN7dSrfnjIxSsHlOAryAwRZRGfnk5QoMtDi2c qCDpew0MDxHU7BpwsvvhSN1Rpe+mPq7RL0Kak1KDn/keNdfsWL4RVsa6bUmWh8DrjU7E0qvRHkr eB7J1cEgoqDDGCwI4peDNHoX4h/Q6bl5xejOXCA== X-Google-Smtp-Source: AGHT+IE95hfuzepLpoZGWH+zW4Ph52ptBMCm//EMdBRDvUNVuAplouPpCwidOSX/oqz4/Uv1s023VGgTeJFyfcvUNvI= X-Received: by 2002:a17:90a:d348:b0:2c2:d8da:b9ba with SMTP id 98e67ed59e1d1-2c4db24d3f1mr3863462a91.20.1718396975688; Fri, 14 Jun 2024 13:29:35 -0700 (PDT) MIME-Version: 1.0 References: <20240611161606.23881-2-dmarx@iol.unh.edu> <20240614150238.26374-1-dmarx@iol.unh.edu> <20240614150238.26374-2-dmarx@iol.unh.edu> In-Reply-To: From: Jeremy Spewock Date: Fri, 14 Jun 2024 16:29:24 -0400 Message-ID: Subject: Re: [PATCH v3 1/3] Added VLAN commands to testpmd_shell class To: Patrick Robb Cc: Dean Marx , Honnappa.Nagarahalli@arm.com, juraj.linkes@pantheon.tech, paul.szczepanek@arm.com, yoan.picchi@foss.arm.com, bruce.richardson@intel.com, luca.vizzarro@arm.com, dev@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Fri, Jun 14, 2024 at 12:00=E2=80=AFPM Patrick Robb w= rote: > > On Fri, Jun 14, 2024 at 11:03=E2=80=AFAM Dean Marx wr= ote: > > + def vlan_filter_set_on(self, port: int =3D 0, verify: bool =3D Tru= e): > > + """Set vlan filter on. > > + > > + Args: > > + port: The port number to use, should be within 0-32. > > + verify: If :data:`True`, the output of the command is scan= ned to verify that > > + vlan filtering was enabled successfully. If not, it is > > + considered an error. > > + > > + Raises: > > + InteractiveCommandExecutionError: If `verify` is :data:`Tr= ue` and the filter > > + fails to update. > > + """ > > + filter_cmd_output =3D self.send_command(f"vlan set filter on {= port}") > > I wonder whether, when convenient, we want to name the methods more or > less 1:1 according to the actual testpmd text command they send? I.e. > in this case should the method be named vlan_set_filter_on instead of > vlan_filter_set_on (aligns better with "vlan set filter on {port}")? > The intellisense provided by the testpmd methods is indeed a QoL > improvement for folks writing testsuites, but at the same time people > who use testpmd will always have the real commands ingrained in their > thoughts, so if we try to stay as true to those as possible, we get > the stability and intellisense and also the method names are still > intuitive. I generally try to name these methods in a more human-readable way, so my intuition would lean more towards naming it something like `set_vlan_filter_on` or `set_vlan_strip_on`. I could see how it might make it easier for some testpmd developers, so I'm not sure which would be better. Personally, as someone who is less familiar with all the ins and outs of testpmd, I prefer the human-readable approach. > > Maybe even think tiny things like renaming the method set_forward_mode > to set_fwd_mode to align 1:1 with testpmd is good. > > That's just my perspective though - I would be interested to see > whether others feel the same or not. > > > 2.44.0 > >