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

make pylint happy #41

Merged
merged 2 commits into from
Nov 13, 2014
Merged

make pylint happy #41

merged 2 commits into from
Nov 13, 2014

Conversation

gurneyalex
Copy link
Member

No description provided.

@yvaucher
Copy link
Member

👍

@@ -103,14 +103,14 @@ def create_quotation(test, requisition, lines):
sale_id = res['res_id']
sale = test.env['sale.order'].browse(sale_id)
sale_lines = sale.order_line
source_lines = [sl for line in lines for sl in line.source_ids]
_source_lines = [sl for line in lines for sl in line.source_ids]
Copy link
Member

Choose a reason for hiding this comment

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

What's the explanation for this? Thank you in advance.

Copy link
Member

Choose a reason for hiding this comment

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

@tafaRU source_lines is already a method name upper in this file.

I guess that @gurneyalex didn't wanted to break a method signature that could impact other modules.

Copy link
Member Author

Choose a reason for hiding this comment

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

with the new pylint config, you get an error because source_lines is shadowing a function with the same name in the file.

I am not happy with the new travis config, but right now, I really need a green build.

Copy link
Member

Choose a reason for hiding this comment

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

Shadowed variables can result in side effects, so this prevents it. Now it's a pain to greenify all, I agree.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 540f6ce on gurneyalex:8.0-pylint into * on OCA:8.0*.

@pedrobaeza
Copy link
Member

👍

Fast tracked!

pedrobaeza added a commit that referenced this pull request Nov 13, 2014
@pedrobaeza pedrobaeza merged commit db8952c into OCA:8.0 Nov 13, 2014
@tafaRU
Copy link
Member

tafaRU commented Nov 13, 2014

@yvaucher @gurneyalex @pedrobaeza thanks so much for your explanations! This is what It is called "Power of Community" 💪 and I love it 💚

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

5 participants