[REF] pylint_beta.cfg: Removing beta messages to enable real error

[moylop260]

This PR enable next configuration checks:

Message	PR or branch error	Affect build status?	10.0	9.0	8.0	7.0	6.1
api-one-multi-together	PR	yes	yes	yes	yes	n/a	n/a
class-camelcase	PR	yes	yes	yes	no	no
create-user-wo-reset-password	PR	yes	yes	yes	yes	yes
consider-merging-classes-inherited	PR	yes	yes	yes	yes	yes
dangerous-filter-wo-user	PR	yes	yes	yes	yes	yes	yes
dangerous-view-replace-wo-priority	PR	yes	yes	yes	yes	yes	yes
duplicate-id-csv	PR	yes	yes	yes	yes	yes	yes
duplicate-xml-record-id	PR	yes	yes	yes	yes	yes	yes
incoherent-interpreter-exec-perm	PR	yes	yes	yes	yes	yes	yes
manifest-author-string	PR	yes	yes	yes	yes	yes	yes
manifest-deprecated-key	PR	yes	yes	yes	yes	no	no
manifest-required-author	PR	yes	yes	yes	yes	yes	yes
manifest-required-key	PR	yes	yes	yes	yes	yes	yes
missing-readme	PR	yes	yes	yes	yes	no	no
no-utf8-coding-comment	PR	yes	yes	yes	yes	yes	yes
odoo-addons-relative-import	PR	yes	yes	yes	yes	yes	yes
openerp-exception-warning	PR	yes	yes	n/a	yes	n/a	n/a
rst-syntax-error	PR	yes	yes	yes	yes	yes	yes
translation-field	PR	yes	yes	yes	yes	yes	yes
translation-required	PR	yes	yes	yes	yes	yes	yes
use-vim-comment	PR	yes	yes	yes	yes	yes	yes
wrong-tabs-instead-of-spaces	PR	yes	yes	yes	yes	yes	yes

[pedrobaeza]

For what branches is this?

[moylop260]

all branches just modules changed

[moylop260]

With exceptions in
https://github.com/OCA/maintainer-quality-tools/blob/master/travis/cfg/travis_run_pylint_exclude_61.cfg
https://github.com/OCA/maintainer-quality-tools/blob/master/travis/cfg/travis_run_pylint_exclude_70.cfg

[lmignon]

@moylop260 does that mean that if I change 1 line in a file in a v8 addons Travis will fail if one of these new checks fails?

[moylop260]

@lmignon
Yes, that is right.
If you change a module (one line) you will have a red status.

The idea of beta messages is that one time is tested, now should be stable messages.
By now just in modules changed.

After that (4 months, 6 months... I don't know), in all modules of branch.

FYI The proposal of active them is from next community thread

[dreispt]

@moylop260 Alexandre proposed that for 9.0 only. I'm not sure if we should activate that for 8.0. It certainly shouldn't be activated for < 8.0.

Can you point at the doc explaining how the different cfg files are used? I'm not finding it.

[moylop260]

@dreispt
You are right! but why not in < v8.0 too?
(FYI we don't have a beta.cfg by version)

The idea is that beta.cfg should be exists only to test them, then now are stable messages and we should remove it from beta.cfg in a moment (Maybe that moment is now with migration v9)

Can you point at the doc explaining how the different cfg files are used? I'm not finding it.

https://github.com/OCA/maintainer-quality-tools/blob/master/travis/test_pylint#L5-L43

[moylop260]

@ALL
Can you check result of messages enabled in builds where PULL REQUEST is activated with version 7.0 and master?

Build-7 - For master version
Build-8 - For 7.0 version

[moylop260]

See main message with information of new configuration checks

[pedrobaeza]

Thanks for the clarification, 👍

[moylop260]

Thanks @pedrobaeza

[moylop260]

@gurneyalex
This PR is your request.
You feedback is welcome

[moylop260]

This PR is step 2: stable process progressive of next comment: #280 (comment)

[moylop260]

@pedrobaeza @lepistone @guewen @gurneyalex @max3903 @dreispt @lmignon
This PR was requested by @gurneyalex but never merged.
I saw a problem, that the configuration files of pylint are very hard to understand and impractical for new versions by the way of the logic of MQT.

I want to propose change the logic to a configuration file by version of odoo:

• Add pylint_{VERSION}.cfg to enable checks of branch base.
• Add pylint_{VERSION}_pr.cfg to enable checks of PR branch (just in modules changed).
• Delete the global pylint configuration files like as: travis_run_pylint.cfg, travis_run_pylint_beta.cfg, travis_run_pylint_pr.cfg
• Delete the particular excludes pylint configuration files like as: travis_run_pylint_exclude_70.cfg, travis_run_pylint_exclude_61.cfg,

Current structure:

• cfg/
  • travis_run_pylint_exclude_61.cfg
  • travis_run_pylint_exclude_70.cfg
  • travis_run_pylint.cfg
  • travis_run_pylint_pr.cfg
  • travis_run_pylint_beta.cfg

New structure:

• cfg/
  • travis_run_pylint_master.cfg
  • travis_run_pylint_master_pr.cfg
  • travis_run_pylint_9.0.cfg
  • travis_run_pylint_9.0_pr.cfg
  • travis_run_pylint_8.0.cfg
  • travis_run_pylint_8.0_pr.cfg
  • travis_run_pylint_7.0.cfg
  • travis_run_pylint_7.0_pr.cfg
  • travis_run_pylint_6.1.cfg
  • travis_run_pylint_6.1_pr.cfg

These is a hard work and I don't want work before to discuss and receive a 👎 after that 😞

What do you think?

[pedrobaeza]

Yes, it can be a good idea to have only one place to see.

[gurneyalex]

👍

[max3903]

I think changes in our process/infrastructure should be implemented for a new version and remains the same for that version.

[pedrobaeza]

@dreispt, this PR enables more checks. @moylop260, can you rebase this one to see if we merge it to allow more checks that make Travis red?

[dreispt]

I think we really really need this.
They should be activated on v=10.0 and on PRs only <10.0.

[lasley]

Regarding W7907(duplicate-xml-fields) - we ran into a false positive earlier, so I'm not so sure it would be good to fail builds on it. I'm trying to figure out what's causing it, but not so sure. Here's the info:

• clouder-community/clouder#134
• Failure in Travis
• The code

The duplicate XML is being triggered for the name fields in *2M relation.

[moylop260]

@lasley
You have a good point here.
I have reported the following issue OCA/pylint-odoo#76 and I added to beta messages this one again

[lasley]

Question regarding manifest-required-author - is it our intent to disallow the use of these tools for repos not controlled by OCA?

[moylop260]

For custom projects you could use a custom configuration file.
For our projects we are using the following pylint.conf

You can disable and enable checks or change parameters.

[lasley]

Forgive my ignorance, but how do you define the pylint for a custom project? Is it a Travis key or something?

It kind of makes me 😢 having to maintain another file, but doesn't affect me enough to try and come up with a solution or block the inclusion of it into the non-beta checks. Just more incentive to move stuff into OCA, really.

[moylop260]

Define a parameter is a cool feature. (It's not created)
I have a patched MQT to overwrite the current pylint.conf files with wget: travis_install_nightly.patched

[lmignon]

Ok ONLY for v10
NOk for PRs < v10
IMO It's unacceptable that these new checks blocks PRs on < v10. Continuous changes in these rules discourage new contributions. ..

[moylop260]

Really we aren't adding new checks.
This checks exists from 1 year ago and we are creating manual comments "you have beta messages..."
This pr dont add new checks just avoid use the manual comment.
After a year emitting the messages It means today there arent those fails.

[nhomar]

@moylop260 Should not we close this PR or it is valid yet?

[dreispt]

@MOYLOP you are assuming that people look at the CI log for the beta messages.
I agree that new checks should be introduced only at new versions, such as v10 or v11.

[lasley]

I agree that new checks should be introduced only at new versions, such as v10 or v11.

It's sad because that will leave old supported versions out of the new test bandwagon, but assuming we keep the output in the logs I think this is the best solution as well. Otherwise we will find ourselves rewriting modules to perform simple one line bugfixes