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 89E4B431EC; Tue, 24 Oct 2023 08:40:06 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4DF72402B7; Tue, 24 Oct 2023 08:40:06 +0200 (CEST) Received: from mail-ed1-f51.google.com (mail-ed1-f51.google.com [209.85.208.51]) by mails.dpdk.org (Postfix) with ESMTP id D1C9E4021D for ; Tue, 24 Oct 2023 08:40:04 +0200 (CEST) Received: by mail-ed1-f51.google.com with SMTP id 4fb4d7f45d1cf-53e70b0a218so6111370a12.2 for ; Mon, 23 Oct 2023 23:40:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1698129603; x=1698734403; 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=TWWprAH2nBBcBlLHG27rB1aL6VJ6TdOIaADe8RGL/P8=; b=uheuX2ZWMHVlIQ4ntoBFA5c2edkg3EfxugBhcLmP5bfMaq3diLDvBv+Iib2y27K6YX g9BbeLSADNYI6aR9Mfz5ApUvfvGQ7XBeiTS79qXNBAyKIn81ELbf8bbzQaWJGyOWlJsB aW8M5RQsgwwvWUz5SV0G1pM6yjfCxJL8k0VVVwmG10z8fVp5fCB58hTX9jRG07RZsEsn ZaGu5wbR126fXOMHaSw0nJaALk3tSv8/nEnYR6A8C84dU+1YLeOzSvF5Mkm1X47JjkXi Y6dd4tkmmWUtLfaXAisMStBOBW5flMMPV4SlCCXPEsjZW0bSLPt4jUaWWUCbkvF81M/7 DUvw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698129603; x=1698734403; 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=TWWprAH2nBBcBlLHG27rB1aL6VJ6TdOIaADe8RGL/P8=; b=jnxoDi6Sl4qZEFiRUK0npShl7ttJfiO3UZRMfY5sxYJioXR1EqWRvAUSoEkS0exvtS 69VkaSlcXhORoTAsqPQoOlByndGY/cOneLNRaZaoYtrwrCeWUjmjRaQ3foW/m53KKMpY rLinguAuDt34hn0m0yDLt7bqx/i4AVjQ4PuUnqM6ujev1lcUiP5bpyAawhHJeGJjTAak V+8zr/sbYE3IVe79CIH6rolp4qhiwou5gDKjda6uIriWVpstssU3WNUMHuLX3DqQsOIo hGCuqmnnvh+cEvtwUANih4u0rtfJesFDsfxNK6t6w4gEvze2m/Xewy5NZsVZfkFT7oIT SouA== X-Gm-Message-State: AOJu0Ywgag7UDT49N7dyzSsL29b6aYiQLEbeIxYqSrs+M01jdA/zs/Sj 71dbkltY4uMBfOB47ZchRh+KC1UyufIJlLZNlnB6NQSVa7XrfmZIKq85WA== X-Google-Smtp-Source: AGHT+IGQQ+TXYjM+S3Md5dSKn5mH/q1ENSOZZ7eFCarlOVRbTnS6sxaN993jXse4MAtDm5ibPQuVOksVA1FEmZ0/urc= X-Received: by 2002:a17:907:3e20:b0:9bf:ff8a:76c8 with SMTP id hp32-20020a1709073e2000b009bfff8a76c8mr8821779ejc.18.1698129603566; Mon, 23 Oct 2023 23:40:03 -0700 (PDT) MIME-Version: 1.0 References: <20230511091408.236638-1-juraj.linkes@pantheon.tech> <20230831100407.59865-1-juraj.linkes@pantheon.tech> <20230831100407.59865-2-juraj.linkes@pantheon.tech> <69c39f25-563e-4e32-a6a2-81e0049332d5@foss.arm.com> In-Reply-To: From: =?UTF-8?Q?Juraj_Linke=C5=A1?= Date: Tue, 24 Oct 2023 08:39:52 +0200 Message-ID: Subject: Re: [RFC PATCH v4 1/4] dts: code adjustments for sphinx To: Yoan Picchi Cc: thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, bruce.richardson@intel.com, jspewock@iol.unh.edu, probb@iol.unh.edu, 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 Mon, Oct 23, 2023 at 1:52=E2=80=AFPM Yoan Picchi wrote: > > On 10/23/23 07:44, Juraj Linke=C5=A1 wrote: > > > > > >> > >> My only nitpick comment would be on the name of the file common.py tha= t > >> only contain the MesonArgs class. Looks good otherwise > > > > Could you elaborate a bit more, Yoan? The common.py module is supposed > > to be extended with code common to all other modules in the > > testbed_model package. Right now we only have MesonArgs which fits in > > common.py, but we could also move something else into common.py. We > > also could rename common.py to something else, but then the above > > purpose would not be clear. > > > > I'm finishing the docstrings soon so expect a new version where things > > like these will be clearer. :-) > > My issue with the name is that it isn't clear what is the purpose of > this file. It only tell to some extend how it is used. Well, the name suggests it's code that's common to other modules, as in code that other modules use, just like the MesonArgs class, which is used in three different modules. I've chosen common.py as that's what some of the DPDK libs (such as EAL) seem to be using for this purpose. Maybe there's a better name though or we could move the class elsewhere. > If we start adding more things in this file, then I see two options: > - Either this is related to the current class, and thus the file = could > be named meson_arg or something along those lines. > - Or it is unrelated to the current class, and we end up with a f= ile > coalescing random bits of code, which is usually a bit dirty in OOP. > It's code that's reused in multiple places, I'm not sure whether that qualifies as random bits of code. It could be in os_session.py (as that would work import-wise), but that's not a good place to put it, as the logic is actually utilized in sut_node.py. But putting it into sut_node.py doesn't work because of imports. Maybe we could just put it into utils.py in the framework dir, which is a very similar file, if not the same. My original thoughts were to have a file with common code in each package (if needed), depending on where the code is used (package level-wise), but it looks like we can just have this sort of common utilities on the top level. > Like I said, it's a bit of a nitpick, but given it's an RFC I hope > you'll give it a thought in the next version. I thought a lot about this before submitting this RFC, but I wanted someone to have a look at this exact thing - whether the common.py file makes sense and what is the better name, common.py or utils.py (which is why I have both in this patch). I'll move the MesonArgs class to the top level utils.py and remove the common.py file as that makes the most sense to me now. If you have any recommendations we may be able to make this even better.