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

[ADD] attribute-string-redundant: Check if the string parameter is eq… #100

Conversation

JesusZapata
Copy link
Member

@JesusZapata JesusZapata commented Dec 29, 2016

Check if the name of the variable is equal to string parameter

Tested cases

name_equal_to_string = fields.Float("Name equal to string") # Show warning
many_2_one = fields.Many2one('res.users', "Many 2 one") # Show warning
many_2_many = fields.Many2many('res.users', 'relation', 'fk_column_from', 'fk_column_to', "Many 2 many") # Show warning

This change is supported by CONTRIBUTING guide of oca

If the technical name of the field (the variable name) is the same as the string of the label, do not put string parameter for new API fields, because it's automatically taken. If your variable name contains "_" in the name, they are converted to spaces when creating the automatic string and each word is capitalized. (Example: old api 'name': fields.char ('Name', ...) new api 'name': fields.Char (...))

https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md#fields

@oca-clabot
Copy link

Hey @JesusZapata, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla
Here is a list of the users:

Appreciation of efforts,
OCA CLAbot

@moylop260
Copy link
Collaborator

Please update this pr with last changes and fixes
And fix conflicts

@JesusZapata JesusZapata force-pushed the master-oca-same-field-name-with-string-jesuszapata branch from 617cb2a to a8a0dc3 Compare January 26, 2017 14:53
@JesusZapata
Copy link
Member Author

@moylop260
Done!
I applied this solution Vauxoo#108 in this PR

@moylop260
Copy link
Collaborator

Sorry new conflicts

@JesusZapata
Copy link
Member Author

@moylop260
Done! I fixed this conflict!

@moylop260
Copy link
Collaborator

@lasley @pedrobaeza @dreispt @lmignon
your feedback is welcome

Copy link
Contributor

@lasley lasley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ❤️ it!

It will be nice to be able to remove this from the mental checklist of things to look for during PR reviews - it's a common mistake too, particularly from older code.

@moylop260 moylop260 merged commit 4fa6c37 into OCA:master Jan 27, 2017
@moylop260 moylop260 deleted the master-oca-same-field-name-with-string-jesuszapata branch January 27, 2017 18:30
@oca-clabot
Copy link

Hey @JesusZapata,
We acknowledge that the following users have signed our Contributor License Agreement:

Appreciation of efforts,
OCA CLAbot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants