[RFC PATCH v4 1/4] dts: code adjustments for sphinx

[Yoan Picchi]

On 10/24/23 07:39, Juraj Linkeš wrote:
> On Mon, Oct 23, 2023 at 1:52 PM Yoan Picchi <yoan.picchi at foss.arm.com> wrote:
>>
>> On 10/23/23 07:44, Juraj Linkeš wrote:
>>> <snip>
>>>
>>>>
>>>> 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.
> 
> 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 file
>> 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.

I didn't meant to imply you did not think a lot about it, sorry if it 
came that way.
I prefer the idea of utils.py to a common.py, be it at package level or 
above. There might also be the option of __init__.py but I'm not sure 
about it.
That being said, I'm relatively new to dpdk and didn't know common.py 
was a common thing in EAL so I'll leave it up to you.