search

Vauxoo / pylint-odoo

1 stars 4 forks source link

[FIX] pylint_odoo: Detect the children inside the odoo xml #129

Closed JesusZapata closed 7 years ago

JesusZapata commented 7 years ago

Detect the children inside the odoo xml tag and only active the check deprecated-data-xml-node when exist one tag data inside de xml

This fix skip the follow case

<odoo>
    <data noupdate="1">
        <record></record>
    </data>
    <record></record>
</odoo>

Fix https://github.com/Vauxoo/pylint-odoo/issues/128

JesusZapata commented 7 years ago

@moylop260 This is a small change to avoid false positives

moylop260 commented 7 years ago

I'm not sure that this is a valid xml for odoo. I mean, official documentation talk us about use both.

Could you check if odoo use the noupdate correctly for this case?

JesusZapata commented 7 years ago

@moylop260 In the documentation of odoo I can't no see noting related with the use of noupdate

Only see the follow help related to noupdate The tag <data> is only used to set not-updatable data with noupdate=1

And I found the same behavior on the odoo code

https://github.com/odoo/odoo/blob/3ad1867ea4d7fcda4ccfeb0a7f3d3860b16d389e/addons/sales_team/data/sales_team_data.xml#L13-L18

https://github.com/odoo/odoo/blob/55b8a454c97020fa52799651c3312fcebb3ee5d3/addons/marketing_campaign/security/marketing_campaign_security.xml#L13-L16

https://github.com/odoo/odoo/blob/fc2e80cb4bcc450762c7ac5cb82a3e2d88062b38/addons/mass_mailing/security/mass_mailing_security.xml#L13-L17

https://github.com/odoo/odoo/blob/fc2e80cb4bcc450762c7ac5cb82a3e2d88062b38/addons/im_livechat/security/im_livechat_channel_security.xml#L20-L25

JesusZapata commented 7 years ago

@moylop260

I run this change over the module of odoo sales_team

Without this change the output is the follow image The pylint-odoo detect four files

After apply this change over the code the output is the follow image The pylint-odoo detect two files

moylop260 commented 7 years ago

Could you create the OCA's PR, please? I'll update this project after merge

JesusZapata commented 7 years ago

I will close this pull request because the pull request in the OCA was merged https://github.com/OCA/pylint-odoo/pull/156 and the change is in the Vauxoo since https://github.com/Vauxoo/pylint-odoo/commit/43a5af59104e9a8cb79396fafde74047acc37522