Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REF] pylint: Enable check to validate CamelCase class name for odoo version >= 8.0 #233

Closed
wants to merge 2 commits into from

Conversation

moylop260
Copy link
Contributor

Progressive PR from #232
To review the real diff of this PR see last commit.

This PR enable check to validate CamelCase class name just for modules changed and just for modules of odoo version >= 8.0

@pedrobaeza
Copy link
Member

You don't need to depends on vauxoo-dev@ed542ce for this one, doesn't it?

@moylop260
Copy link
Contributor Author

@pedrobaeza
I need it because this is a new conventions

@nhomar
Copy link
Member

nhomar commented Aug 18, 2015

2015-08-18 12:58 GMT-05:00 moylop260 notifications@github.com:

I need it because this is a new conventions

@pedrobaeza.

  1. Have the script does not mean we will run it all at once without take
    care.
  2. You need to decide, if we will have the rule we ned automate it, it is
    more risky continue expending this job manually to all collaborators as I
    exposed before.
  3. We already test them in our repositories and we passed some iterations
    already.

Then block this PR just becuase a non fundamented fear is not correct.

  • It has unit tests to vaildate regresions.
  • If I have a repository which call a class with wrong names will be red
    BEFORE the PR is excecuted, and ALL the manual checks that we are doing
    will be done automatically....
  • The miogration is manual we are only giving a tool, not automating the
    conversion itself...

What is exactly the situations where you think it can fails in order to
include such unit tests and considere in the code?

BUt a Generic "too dangerous" is not a good and productive incentive
here.... con you please be more precise?


Saludos Cordiales

CEO at Vauxoo https://www.vauxoo.com Odoo's Gold Partner.

[image: --]
Nhomar Hernandez
[image: http://]about.me/nhomar
http://about.me/nhomar?promo=email_sig

@pedrobaeza
Copy link
Member

OK, I understand. Let's follow this path then.

@moylop260 moylop260 self-assigned this Aug 18, 2015
@moylop260 moylop260 force-pushed the oca-pylint-camelcase-moy branch 2 times, most recently from a84855a to c87d790 Compare August 22, 2015 02:09
@dreispt
Copy link
Sponsor Member

dreispt commented Aug 28, 2015

This depends on #232 and most of the code is already there. instead of "Needs Review" It would be better marked as "WIP" (though it would be nice t have a tag for "Wainting on Dependencies"...).

@moylop260
Copy link
Contributor Author

I promised to @lepistone add message clearer.
This PR check CamelCase but just show invalid-name

Now, with next PR #234 we have next message:
Use CamelCase "%s" in class name "%s". You can use oca-autopep8 of https://github.com/OCA/maintainer-tools to auto fix it.

Then I close without merge this one.

@moylop260 moylop260 closed this Sep 4, 2015
moylop260 pushed a commit that referenced this pull request Sep 19, 2017
…ver the weblate-ssh server (#484)

* [FIX] apis.py: Adding new metod _ssh_keyscan to run the ssh-keyscan over the weblate-ssh server (#233)

* [REF] broken_deprecated: Use correct version format
 - After merge OCA/pylint-odoo#155 we had this unexpected wrong format version
bt-admin pushed a commit to brain-tec/maintainer-quality-tools that referenced this pull request Oct 20, 2017
…ver the weblate-ssh server (OCA#484)

* [FIX] apis.py: Adding new metod _ssh_keyscan to run the ssh-keyscan over the weblate-ssh server (OCA#233)

* [REF] broken_deprecated: Use correct version format
 - After merge OCA/pylint-odoo#155 we had this unexpected wrong format version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants