On 1/13/07, goff <[EMAIL PROTECTED]> wrote:
Please have a look at http://docs.turbogears.org/1.0/UsingPylintToImproveYourCodesQuality I linked it from 1.0/RoughDoc' Uncatoried section. mfg
Some contrarian opinions: --disable-msg=W0401 - wild card imports wild card imports are a hack and only truly meant for use in an interactive shell. The strawman argument proposed is just that. I've refactored my own projects model.py to lint out at 10 (default config) and here are model.py imports in full: from datetime import datetime from turbogears.database import PackageHub import sqlobject as orm from turbogears import identity You then just preface those SO or SA methods/classes with orm., not much more typing and if you have a sizable project it will help steer you around nasty unintended name space problems. (personally, I am bewildered by the hubris of package writers who just assume that you should wildcard "their" package. It's the only way you see it used in the "their" documentation) Rhetorically, do you recall how much total time you put in to figuring out where all of these magic keywords/methods were appearing from in model.py? What if two different packages were wild carded in to the module, things get a bit fuzzier, how about 3 -- where do you draw the line? I argue that 0 is where the line should be drawn. If wild carding is such a good idea, why don't we see "from turbogears import *" --disable-msg=W0611 - unused imports This is a useful tool, especially if you want to distribute your code. Why waste effort packaging imports that you don't need. It is just cruft left around from exploratory coding or refactoring and you are too lazy to clean up. Going forward it confuses newbies who blindly cut and paste the imports and perpetuate the problem. --disable-msg=E0602 - undefined variable is accessed No "Error" level warning should be disabled as a general rule -- which is what the wiki should be suggesting. --disable-msg=R0201 - bound (walks like a function, talks like a function then just maybe...) This message, in my experience, is a good thing. When designing my last project it was going off on some of my classes. However, once I thought it out, those classes served no good reason and the use of a function was more direct and more proper. Sometimes we get caught up in objects and forget that sometimes, mind you sometimes, a function is called for and not a class. --indent=$'\t' - pep8 strongly recommends spaces and practically requires them going forward and pep8 is the guideline for TG
From the PEP, "Tabs or Spaces?
Never mix tabs and spaces. The most popular way of indenting Python is with spaces only. The second-most popular way is with tabs only. Code indented with a mixture of tabs and spaces should be converted to using spaces exclusively. When invoking the Python command line interpreter with the -t option, it issues warnings about code that illegally mixes tabs and spaces. When using -tt these warnings become errors. These options are highly recommended! For new projects, spaces-only are strongly recommended over tabs. Most editors have features that make this easy to do." Dont override builtins ( and dont disable W0622) - patch accepted for 1.0.1 so will no longer an issue. This deals with the specific issue you mention as I encountered it too. However I submitted a patch to correct it instead of turning off this important warning. http://trac.turbogears.org/ticket/1231 Missing docstrings are an error. imho: This should be filed as a bug with pylint since they list them as a warning<g>. Now in no way should these comments be taken as me finding something wrong with "your" code or way, in my journey with python I've committed every sin too, but we need to focus on best practices going forward for TG. TG should be the gold standard and not just a hack that works. The wiki should list the minimum changes to the default config and stay away from settings that are geared for a particular project or coding style or sweeping under the rug existing problems in the TG code base. Deviations to the default need strong sound reasons otherwise they are just aids in justifying the current state of affairs. If code lints out at a 2 or a 6, then that is how it lints out. Making it return a 10 by turning off useful warnings doesn't change the reality of the code. Also, by turning off fewer defaults, some may check in code at a 3 or 7 instead of a 2 or a 6 and everyone involved will be better off for it. Just because a module doesn't lint out at 10 doesn't make it bad or unacceptable -- it only quantifies the reality of most code, it could be better. If I submit a patch that takes a module that is returning -20.4 and now it's coming in at a 2 -- that is an improvement but it can't be seen nor even noticed if too many warnings/errors/etc are turned off. Code should strive to be come better by being more explicit and better documented. Also, the task or improving the pylint score of existing code is something a second tier contributor, like me, could take on. It is not a task the requires a committer to do, evaluate yes, but the actually refactoring could be delegated. TG gets cleaner code, contributors like us get something meaningful to do that really does contribute to the project and in the process we get to learn the details of TG that interest us. I'll close with an apology for sounding didactic and my favorite coding quotation, "Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." -- Brian W. Kernighan --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "TurboGears" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/turbogears?hl=en -~----------~----~----~----~------~----~------~--~---
