mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 52940: Updated CLI pylint configuration to allow up to 30 local variables.
Date Tue, 18 Oct 2016 18:41:21 GMT


> On Oct. 17, 2016, 6 p.m., Joseph Wu wrote:
> > src/cli_new/pylint.config, line 14
> > <https://reviews.apache.org/r/52940/diff/1/?file=1539774#file1539774line14>
> >
> >     Looks like `__enter_namespaces` in https://reviews.apache.org/r/52953/ is over
the default limit of 15 by one variable...
> >     
> >     I'd say that function needs some helpers, rather than silencing the warning.
> 
> Kevin Klues wrote:
>     Again I didn't just add this patch to suppress the warning (though that's what triggered
it). I think having such a small limit on the number of local variables could prove burdensome
given our pattern for "try/except' around al most every call we make. That said, I agree that
`__enter_namespaces` could be broken up a little bit, so I'll address that in the relevant
patch.
>     
>     Are you suggesting I discard this one too then?

I think 15 is a fairly reasonable limit, even with all the exception catching.  I'd prefer
to keep the warning unless it obviously degrades the code quality.  (In which case, we'd bump
the limit or so.)


- Joseph


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52940/#review152998
-----------------------------------------------------------


On Oct. 17, 2016, 1:11 p.m., Kevin Klues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52940/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2016, 1:11 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5676
>     https://issues.apache.org/jira/browse/MESOS-5676
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Updated CLI pylint configuration to allow up to 30 local variables.
> 
> 
> Diffs
> -----
> 
>   src/cli_new/pylint.config c398220db063d249e6c62cf7e8cd6757e7860630 
> 
> Diff: https://reviews.apache.org/r/52940/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message