vimeo / graph-explorer

A graphite dashboard powered by structured metrics
http://vimeo.github.io/graph-explorer/
Apache License 2.0
1.06k stars 93 forks source link

Various tweaks to be pylint-clean #74

Closed thepaul closed 10 years ago

thepaul commented 10 years ago

(feel free to reject any or all of this with extreme prejudice, it won't hurt my feelings- I just find that keeping a project pylint-clean can help to avoid certain classes of bugs very well.)

None of this should have any effect on functionality, except maybe in the case of handling errors better in a few places. Most of this is:

Adds a "pylint-project.sh" script that runs pylint on everything in the project, excluding third-party code. Could be added to a git commit hook if desired.

Dieterbe commented 10 years ago

interesting how zealous pylint is, given we need to add those #pylin: disable=<...> snippets in various places where the code is perfectly reasonable.

thepaul commented 10 years ago

I find it helps catch a lot of genuine issues. Things like a bare "except Exception:" really are generally a bad idea. There are cases where it's the right thing to do, and the "pylint: disable=" marker is a nice way to inform the reader (and the linter) that you understand the rule and have a good reason for breaking it.

But it's your project. If you think they're annoying, each individual warning can be disabled entirely, or we can leave off pylinting entirely.

Dieterbe commented 10 years ago

this all looks pretty good, just one thing: there's still a lot of plugins that call self.fix_underscores() and self.add_tag() even though those two are now static methods, is that a problem?

thepaul commented 10 years ago

No, there's no problem with getting references to a staticmethod through an instance. It'll still work the same.