Prepare Odoo 15

[sbidoul]

Upgrade linters, while avoiding unnecessary disruption which make fw/backports difficult.

[sbidoul]

@moylop260 et al, can you look at https://github.com/OCA/oca-addons-repo-template/blob/master/src/.pylintrc.jinja and https://github.com/OCA/oca-addons-repo-template/blob/master/src/.pylintrc-mandatory.jinja and suggest new checks to add or move for v15 ?

[moylop260]

Hi @sbidoul thanks for you efforts on this

I just got the diff between the pylint-odoo checks enabled in the configuration file vs all the current ones in pylint-odoo

The diff are the following checks:

• attribute-string-redundant
• character-not-valid-in-resource-link
• consider-merging-classes-inherited
• context-overridden
• copy-wo-api-one
  • It is not valid for new versions
• create-user-wo-reset-password
• dangerous-filter-wo-user
• dangerous-qweb-replace-wo-priority
• deprecated-data-xml-node
• deprecated-openerp-xml-node
• duplicate-po-message-definition
• except-pass
• file-not-used
• invalid-commit
• javascript-lint
  • It is supported by pre-commit eslint
• manifest-maintainers-list
• missing-import-error
  • It is not needed anymore
• missing-manifest-dependency
  • It is not updated to new way to use pypi packages name directly
• missing-newline-extrafiles
• missing-readme
• missing-return
• no-utf8-coding-comment
  • It is not needed anymore
• odoo-addons-relative-import
• old-api7-method-defined
• po-msgstr-variables
• po-syntax-error
• renamed-field-parameter
• resource-not-exist
• str-format-used
• test-folder-imported
• translation-contains-variable
• translation-positional-used
• unnecessary-utf8-coding-comment
• website-manifest-key-not-valid-uri
• xml-attribute-translatable
• xml-deprecated-qweb-directive
• xml-deprecated-tree-attribute

[pedrobaeza]

cc @Yajo

[yajo]

Upgrade linters, while avoiding unnecessary disruption which make fw/backports difficult.

I wonder if we should start using darker instead of black, to help on this. What do you think?

[pedrobaeza]

I don't think we should add another tool more to the chain, and it's not so critical to always pass pre commit in a separate previous commit.

[moylop260]

There are a few of out-of-the-box pylint checks that could be good to have

• Use a generator instead 'any(field in vals for field in m2m_fields)' for any([field in vals for field in m2m_fields])
• logging-not-lazy Specify string format args as loggin funct parameters.
  • Use _logger.warning('Msg with a param %s' % param1) instead of _logger.warning('Msg with a param %s', param1)
  • A bad parsing won't raise an error since that it only is a logger not logic and the string won't be evaluated early
• consider-using-with opening files with open() as f: f.read() instead of f=open();f.read();f.close()
  • Using with the file will be closed even if there is an exception in the middle
• use-dict-literal my_dict = {} instead of my_dict = dict()
  • More info about https://doughellmann.com/posts/the-performance-impact-of-using-dict-instead-of-in-cpython-2-7-2/
• consider-using-in use var in ('value1', 'value2') instead of var == 'value1' or var == 'value2'
• format-string-without-interpolation "my string without interpolation" % variable
• unnecessary-comprehension use list(range(...)) instead of [i for i in range(...)]
• no-else-continue more info https://github.com/PyCQA/pylint/issues/3574
• self-assigning-variable variable = variable
• trailing-comma-tuple the following code "%s %s" % (value1, value2), notice the last comma shouldn't be used there

[sbidoul]

There are a few of out-of-the-box pylint checks that could be good to have

+1 to add more on 15.0. But not on previous branches to avoid making existing branches red.

[yajo]

All these checks you say @moylop260 should go to mandatory or optional checks?

[moylop260]

Good question

What do you think @sbidoul ?

[sbidoul]

I'm personally happy with adding mandatory checks in v15. We can till move them to optional if they cause problems.

[yajo]

I've taken a look at checks from https://github.com/OCA/oca-addons-repo-template/issues/69#issuecomment-934556722 and I've been reviewing also those from https://docs.pylint.org/en/latest/technical_reference/features.html.

I feel like we could enable the full W, R and E categories as optional checks. Since those are optional, they could be added to old versions too. WDYT?

[sbidoul]

I feel like we could enable the full W, R and E categories as optional checks. Since those are optional, they could be added to old versions too. WDYT?

In my experience that can becomes very noisy, like for instance the too-many-* R messages. And that could make warnings in the optional checks so numerous that people will not look at them anymore. I think if we want to do that we should at least try on some representative repos to see what it does on real OCA code.

[moylop260]

@Yajo @sbidoul

FYI I'm using a configuration file to enable all the pylint checks then disable what we don't like or not valid for Odoo

This is my list:

• https://github.com/Vauxoo/pylint-conf/blob/043d46066a62a8cb0db12ebb680c105a24397777/conf/pylint_vauxoo_light.cfg#L21-L82

I just share the information no action required here

[sbidoul]

@moylop260 thanks for sharing. At Acsone we also enable all and disable these by default.

[yajo]

Yikes, OK, let's leave pylint as it was 😆

We should specify a process of adding new linters in stable versions, such as adding them to optional and then, when next release is done, add them as mandatory. For the sake of simplicity, let's focus on supporting Odoo 15.0, and let's talk about this later.