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 ED66C431E1; Mon, 23 Oct 2023 13:52:37 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BB77E40A77; Mon, 23 Oct 2023 13:52:37 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id DD7B240270 for ; Mon, 23 Oct 2023 13:52:35 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4BCF31FB; Mon, 23 Oct 2023 04:53:16 -0700 (PDT) Received: from [10.57.4.127] (unknown [10.57.4.127]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 195413F762; Mon, 23 Oct 2023 04:52:33 -0700 (PDT) Message-ID: Date: Mon, 23 Oct 2023 12:52:33 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH v4 1/4] dts: code adjustments for sphinx Content-Language: en-US To: =?UTF-8?Q?Juraj_Linke=C5=A1?= Cc: thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, lijuan.tu@intel.com, bruce.richardson@intel.com, jspewock@iol.unh.edu, probb@iol.unh.edu, dev@dpdk.org 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> From: Yoan Picchi In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 10/23/23 07:44, Juraj Linkeš wrote: > > >> >> My only nitpick comment would be on the name of the file common.py that >> 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. 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 file coalescing random bits of code, which is usually a bit dirty in OOP. 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.