module_analysis does not count code lines from odoo core when installed from the debian package

fcayre commented 1 week ago

Module

module_analysis

Describe the bug

To Reproduce

Affected versions: all

Steps to reproduce the behavior:

Prerequisites: Odoo is installed using the debian package; module_analysis is installed
Go to the account_cancel module page (in Applications app) in debug mode
Press the "Refresh Code Analysis" button in the "Technical data" tab

Expected behavior The Python Code Quantity field's value is 0 also it should not, as there are non-empty python files in the module (models/account_bank_statement.py).

Additional context Debugging shows that this line is a true condition when odoo is installed by a debian package (root = /usr/lib/python3/dist-packages/odoo/addons/account_cancel), because exclude_directories value is ['lib', 'demo', 'test', 'tests', 'doc', 'description'], so the test returns {'lib'}

The exclude_directories comes from ir.config_parameter module_analysis.exclude_directories which interpretation is questionable. As a user of the module I expect its value to list folders relative to the module's folder, while in the code if any of the listed folder is found in the module's root path the entire module will be ignored.

I'm working on a PR to fix this.

fcayre commented 1 week ago

@legalsylvain any thoughts on the module_analysis.exclude_directories parameter interpretation? Am I right and can I go on with the PR?

legalsylvain commented 1 week ago

Hi @fcayre. Thanks for the investigation, regarding your problem. Well, why that default values ? It is about what you want to measure.

In my thought, I wanted to measure the "amount of algorithmic complexity to be maintained".

For that reasons :

I proposed to ignore test / tests files that sometimes add a lot of code, and because if a module has a lot of tests, it is not less maintainable than a module that doesn't have any test. (in my opinion, it's even a bit the opposite)
Similar reasoning for doc / description and demo folders. It improves quality of modules. (better understanding of the module, easy to test for functional people, etc...)
in Odoo, "lib" folder contains extra libraries that comes from other projects. typical exemple, extra js lib used by "web" modules. https://github.com/odoo/odoo/tree/16.0/addons/web/static/lib. Just because the web module depends on jquery doesn't mean it adds complexity (the technical debt is carried by the jquery development team's maintenance team). Then because it wouldn't be very ‘fair’: adding a dependency to a python library adds a line to a requirements.txt file, so adding a javascript dependency should have the same behaviour. (no impact on the measure)

you can see the original PR, with some thought about what should contains or not the "excude_directories" values : https://github.com/OCA/server-tools/pull/1618#issuecomment-509770796

my opinion : people want to measure differents things, and all values is legit. Finally, it is just a default value. So, I propose to not change the default values, but add a text in the configure.rst file, to explain how to configure the module in case you install odoo core via debian, with a link to thisi current issue. If agree could you make the PR in the last recent version of the module, so the text will be conserved when migrating the module. https://github.com/OCA/server-tools/tree/17.0/module_analysis

fcayre commented 1 week ago

@legalsylvain I don't want to change the default value (which looks good to me) but the code, which does not interpret that value as it should in my opinion. See my PR for a better understanding of my objection.

OCA / server-tools