Re: [PATCH 1/2] xen/misra: add diff-report.py tool

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.05.2023 13:10, Luca Fancellu wrote:
> 
> 
>> On 19 May 2023, at 11:53, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>
>> On 19.05.2023 11:46, Luca Fancellu wrote:
>>> Add a new tool, diff-report.py that can be used to make diff between
>>> reports generated by xen-analysis.py tool.
>>> Currently this tool supports the Xen cppcheck text report format in
>>> its operations.
>>>
>>> The tool prints every finding that is in the report passed with -r
>>> (check report) which is not in the report passed with -b (baseline).
>>>
>>> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
>>> ---
>>> Changes from v1:
>>> - removed 2 method from class ReportEntry that landed there by a
>>>   mistake on rebase.
>>> - Made the script compatible also with python2 (Stefano)
>>> ---
>>> xen/scripts/diff-report.py                    |  80 ++++++++++++++
>>> .../xen_analysis/diff_tool/__init__.py        |   0
>>> .../xen_analysis/diff_tool/cppcheck_report.py |  44 ++++++++
>>> xen/scripts/xen_analysis/diff_tool/debug.py   |  40 +++++++
>>> xen/scripts/xen_analysis/diff_tool/report.py  | 100 ++++++++++++++++++
>>> 5 files changed, 264 insertions(+)
>>> create mode 100755 xen/scripts/diff-report.py
>>> create mode 100644 xen/scripts/xen_analysis/diff_tool/__init__.py
>>> create mode 100644 xen/scripts/xen_analysis/diff_tool/cppcheck_report.py
>>> create mode 100644 xen/scripts/xen_analysis/diff_tool/debug.py
>>> create mode 100644 xen/scripts/xen_analysis/diff_tool/report.py
>>
>> If I'm not mistaken Python has no issue with dashes in path names.
>> Hence it would once again be better if the underscores were avoided
>> in the new directory names.
> 
> From what I know python can’t use import for module with dashes in the name, 
> unless
> using some tricks, but if anyone knows more about that please correct me if 
> I’m wrong.
> 
> The style guide for python 
> (https://peps.python.org/pep-0008/#package-and-module-names)
> Says:
> 
> Modules should have short, all-lowercase names. Underscores can be used in 
> the module
> name if it improves readability. Python packages should also have short, 
> all-lowercase names,
> although the use of underscores is discouraged.

Hmm, I was initially thinking there might be such a restriction, but
the I checked a pretty old installation and found plat-linux2/ there
with several .py / .pyo / .pyc files underneath. Which suggested to
me that, for them to be of any use, such a path name must be permitted.

But well, if underscores are required to be used if any separator
is wanted, so be it. Albeit ...

> So, yes, the use is discouraged, but here I think it improves the 
> readability. Unless we want
> to use “difftool” instead of “diff_tool” and “cppcheckreport” instead of 
> “cppcheck_report”.

... personally I'd like both shorter variants better, plus perhaps
xen_ dropped from xen_analysis, or some different name used there
altogether (to me this name doesn't really tell me what to expect
there, but maybe that's indeed just me).

> Can I ask the reason why we need to avoid underscores in file names?

First of all they're odd, a space or dash is simply more natural to
use. From my pov they ought to be used only when a visual separator
is wanted, but neither space nor dash fit the purpose (e.g. for
lexical reasons in programming languages). Plus typing them requires,
on all keyboards I'm aware of, <shift> to be used when dash doesn't.

Jan