search

Vauxoo / pylint-odoo

1 stars 4 forks source link

[FIX] duplicate-xml-fields: False red using 2 tree sub-views *2M fields #93

Closed hugho-ad closed 7 years ago

hugho-ad commented 7 years ago

FIX https://github.com/OCA/pylint-odoo/issues/76

moylop260 commented 7 years ago

@lasley I'll merge in order to get a unique commit and generate a new pr to OCA project

nhomar commented 7 years ago

On Fri, Dec 23, 2016 at 11:59 AM, Dave Lasley notifications@github.com wrote:

@moylop260 https://github.com/moylop260 I'm not a fan of multiline generators, but it's not so bad in this case because the logical separation is also good visually.

I dunno if McCabe is the only stat that should be relevant here though. I would create an example of something with a lower complexity that is significantly less readable than the explicit cousin, but my pylint doesn't seem to have --max-complexity option. Are you using a beta version or something?

pylint: error: no such option: --max-complexity

Well.

@lasley I am I think it is a matter of PoV but your point is really relevant.

@moylop the "complexity" can be measured but the readability counts (import this and it is mentioned there).

On this very specific case readability is good AND complexity is measurable and more small the I am with @moylop

But maccabe can not be the unique indicator as argument I prefer something with a higher maccabe (in 1 or 2 points) but more readable that somthing les complex but impossible to read....

Regards.

Nhomar Hernandez CEO Vauxoo. Site: http://vauxoo.com Twitter: @nhomar Blog: http://nhomar.com Github User: https://github.com/nhomar Odoo Gold Partner Skype: nhomar00 (Envia mail previo no lo superviso siempre). HangOut: nhomar@vauxoo.com Móvil Venezuela: +58 4144110269 Móvil México: +52 1 4773933942

moylop260 commented 7 years ago

Lets to wait the @lasley proposal to get the best option.

Note for me: We should add a entry to CONTRIBUTING in order to avoid discuss the same matter in each PR.

lasley commented 7 years ago

After second review, I'm in agreement that in this case the multi-line is not detrimental in this instance because the visual line break worked really well with the logic break.

@moylop260 - There are two entries in the Contributing that reference this, I agree we need a bit more clarity:

  • Always favor Readability over conciseness or using the language features or idioms.
  • Use list comprehension, dict comprehension, and basic manipulation using map, filter, sum, ... They make the code more pythonic, easier to read and are generally more efficient

They somewhat conflict though, and then the question becomes what the definition of good is in this case if we were going to try and write it down.

Most of the algorithmic scores will likely rate a comprehension less difficult to understand than nested control statements, but it sounds like we're all in agreement that there's a line we need to draw. I wonder where that is...