On Wed, Jun 06, 2012 at 04:32:59PM -0700, Brock Pytlik wrote:
> On 06/05/12 16:43, Shawn Walker wrote:
> >[snip]
> >
> >----------------------------------------
> >src/modules/misc2.py
> >----------------------------------------
> > Not thrilled about having a separate module only because of
> >pylinting :-(
> I'm not ok with this. Please either
> 1) just move the code in misc2.py into misc.py
> 2) or do 1 and make misc.py pylint clean (preferably in another changeset)
> 3) or do 1 and change the pylintrc file so that misc.py is pylint clean
> 4) or do 1 and some combination of 2 and 3
>
i've removed misc2.py and folded the code back into misc.py (this
required fixing some, and disabling other, pylint errors in misc.py)
> Finding things can be difficult as is, and going into misc.py to
> look for something only to find discover that it's been hidden over
> in misc2.py isn't ideal. Not to mention that I believe it also
> increases the change of duplicating names or functions.
>
> When I talked with Ed about this online, he asked that I propose
> changes I'd like to see for our pylintrc file. So I went back and
> looked at what I had done when I looked at making our code work with
> pylint better. Here's what I found:
> The following messages should be disabled:
> # C0302 Too many lines in module
> # R0904 Too many public methods
> # R0911 Too many return statements
> # R0912 Too many branches
> # R0915 Too many statements
> # W0142 Used * or ** magic
> # W0511 Used when a warning note as FIXME or XXX is detected.
> # W0612 Unused variable
> disable-msg=C0302,R0904,R0911,R0912,R0915,W0142,W0511,W0612
>
i've added a bunch (but not all) of these and a few others.
> The argument rgx should be changed to be:
> # Regular expression which should only match correct argument names
> argument-rgx=[a-z_][a-z0-9_]{0,30}$
>
> The variable reg exp should be changed to:
> # Regular expression which should only match correct variable names
> variable-rgx=[a-z_][a-z0-9_]{0,30}$
>
> The const-rgx should be changed to (though that one probably doesn't
> need the 0, 30 on it, and perhaps the others don't either):
> # Regular expression which should only match correct module level names
> const-rgx=(([A-Z_][A-Z1-9_]*)|(__.*__)|([a-z_][a-z0-9_]{0,30}))$
>
these reg-ex are very similar to what we already have and are not
generating any errors so i've left them as is.
> We should also enable init_import which checks for unsed imports in
> __init_files.
>
done.
> The regexp changes above should help with the C0103 errors that the
> code will trigger.
> Those are the changes I'd made. In addition, I'd also suggest (but
> haven't tested):
> R0201 should probably be disabled because it doesn't recognize when,
> because of inheritance, a function needs self as an argument. For
> the same reason, W0613 should be disabled.
i was talking to dan about pylint and he mentioned that there were newer
version available and the newer versions seemed to have fixed bogus
errors that he was seeing. before disabling messages i'd be interesting
to see if that addresses these problems
> Disable :R0902: *Too many instance attributes (%s/%s)*
> Disable :R0913: *Too many arguments (%s/%s)*
i've disabled these (and a few more) of the "too many" warning.
> Either disable W0403 or change the places that use relative imports
> to use absolute imports, I don't really care which.
i think we should leave W0403 enabled, but eliminate unnecessary
relative imports (i've done this in the linked image code). see below
for more comments.
> Either disable W0612 or provide a regexp that we can use to indicate
> to pylint via the variable name that we don't plan on using it. In
> other languages, using '_' represented a purposefully unused
> variable.
>
> For now, I'd leave W0631 enabled, but that would depend on how many
> false positives it generates over the entire code base.
>
i've left these alone for now.
> And finally, I propose that unless some extraordinarily compelling
> reason exists, that no pylint disable commands be allowed into the
> source. Since we control lint, we should either fix the exception,
> turn off the exception, or fix the code.
>
i don't think that's realistic. there are multiple situations that i
can think of (wildcard imports, relative imports, no exception specified
when catching exceptions) that we generally want to avoid, but there
will always be special case there will be exceptions. but having these
errors enabled in pylint will force us to consider which case really
should be special, and otherwise fix the issue. (banning the temporary
disabling of pylint messages would be like saying you can't use LINT
directives in C code. not very realistic.)
ed
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
