search

OCA / pylint-odoo

Odoo plugin for Pylint
http://www.pylint.org
143 stars 168 forks source link

[BUG] method-default prefix is too restrictive #253

Closed lmignon closed 5 years ago

lmignon commented 5 years ago

This new check breaks the code if the provided value for a default is not a method or a lambda.

default='section_1'`
default=lambda m m._default_my_field()
pedrobaeza commented 5 years ago

Example of non-valid error?

lmignon commented 5 years ago

@pedrobaeza An error is raised for the 2 samples provided....


default='section_1'

default=lambda m m._default_my_field()

``
pedrobaeza commented 5 years ago

Uhm, weird I haven't seen that problem on OCA Travis...

@moylop260 any clue?

moylop260 commented 5 years ago

@blaggacao Could you check these valid cases of use for your new check, please?

blaggacao commented 5 years ago

default='section_1'

this is not intended to raise an error.

default=lambda m m._default_my_field()

this is neither... Hm..

blaggacao commented 5 years ago

Shouldn't it be: default=lambda m: m._default_my_field(), though?

blaggacao commented 5 years ago

The most canonical way would be: default=_default_my_field (passed as symbol)

lmignon commented 5 years ago

@blaggacao default=_default_my_field is not overiddable since you pass as symbol... IMO it's not the right way.

blaggacao commented 5 years ago

True it is... :wink:

blaggacao commented 5 years ago

Oh, I see. It was release, I'll take care of a fix asap... :fearful:

lmignon commented 5 years ago

yes it has been released :nauseated_face:

moylop260 commented 5 years ago

So I will revert it and release the revert In order to avoid making false reds for OCA projects

After fixes we can release a new version using all case of use from unittests