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

Red branch because of use builtin names #181

Closed
bealdav opened this issue Apr 2, 2020 · 4 comments
Closed

Red branch because of use builtin names #181

bealdav opened this issue Apr 2, 2020 · 4 comments

Comments

@bealdav
Copy link
Member

bealdav commented Apr 2, 2020

Some branch are red because of code like here in v12

https://travis-ci.org/github/OCA/edi/jobs/663059314#L325-L328


base_ubl/models/ubl.py:57: [W0622(redefined-builtin), BaseUbl._ubl_add_address] Redefining built-in 'zip'

base_ubl/models/ubl.py:286: [W0622(redefined-builtin), BaseUbl._ubl_add_line_item] Redefining built-in 'type'

base_ubl/models/ubl.py:329: [W0622(redefined-builtin), BaseUbl._ubl_add_item] Redefining built-in 'type'

base_ubl/models/ubl.py:609: [W0622(redefined-builtin), BaseUbl.ubl_parse_address] Redefining built-in 'zip'

I propose to rename zip var or arg in zip_ and type in type_

It could break some method calls in case of named argument like here
https://github.com/OCA/edi/blob/12.0/base_ubl/models/ubl.py#L323-L329

but sure we'll at to fix it

What do you think ?

Thanks for sharing your ideas @astirpe @SimoRubi @lmignon @tarteo @alexis-via @joshuajan @feketemihai @acsonefho @rousseldenis @luc-demeyer @Cedric-Pigeon

@bealdav bealdav changed the title Red branch Red branch because of use builtin names Apr 2, 2020
@SimoRubi
Copy link
Member

SimoRubi commented Apr 2, 2020

In my opinion, I'd rather rename the variables/arguments with something that explains their meaning.
In your example

edi/base_ubl/models/ubl.py

Lines 323 to 329 in 742b131

self._ubl_add_item(
name, product, line_item, ns, type=type, seller=seller,
version=version)
@api.model
def _ubl_add_item(
self, name, product, parent_node, ns, type='purchase',

type could be renamed to line_type or something similar.

@astirpe
Copy link
Member

astirpe commented Apr 3, 2020

@bealdav looking at the link to the travis log that you posted, it seems to me that the red branch is caused by the following error only:

************* Module account_e-invoice_generate

account_e-invoice_generate/readme/INSTALL.rst:6: [E7901(rst-syntax-error), ]  Duplicate explicit target name: "oca/edi".

------------


count_errors 1

pylint expected errors 0, found 1!

@bealdav
Copy link
Member Author

bealdav commented Apr 3, 2020

You're right @astirpe
pylint fixed by #186
Could you merge ?

@bealdav
Copy link
Member Author

bealdav commented Apr 14, 2020

OK green now, but we should note to not use builtin names in method for the future

@bealdav bealdav closed this as completed Apr 14, 2020
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

No branches or pull requests

3 participants