On Wed, Nov 24, 2010 at 7:25 PM, Hartmut Goebel
<h.goe...@goebel-consult.de>wrote:
> Hi,
>
Hi,
>
> being new to the list, I just missed the discussion about code quality.
>
Ok, it's always time to got in :)
> Being a quite experiences Python programmer, shinken gets a big "plus"
> because it is written in my favourite programming language.
>
> But ...
>
There always a "but" :)
>
> ... when a had a first glance at the code, is was boggled.
Outch!
> The coding
> style is - well - unusual: No spaces after "#"
I didn't know about the space. I'll try and look how it look (more spacy, so
should be good).
> , lines are far too long,
>
Yes, in fact the main part where we got this is in the "properties", and in
the "if".
For the "properties" dict, it look better wil a uniq line than with multiple
lines. It look more clear I think. I'm not for the "80 characters max" rule
avery where. It's only when it's apply that when it's not. And for
properties it's more clear when it's in one line.
The "if" is a big problem. We've got parameters that are quite long, and so
when you got a or with 3 cases, you are nearly sure to bypass this 80
characters lines :(
comments instead of docstrings,
Yes, this one I know. I do nto like docstrings. I really stry, but somethign
that is between function prototype and code is not a good thing for me. I
came from C, and I see code like :
# comments that say what the function do, the bigs part of
# special things in it, when to call it, and warning about when not
# using it
def my_funtion_name_that_should_not_be_too_long(self, param1, param2,...):
start of my code, with long lines ;)
(space after # seems good, I take it thanks).
Instead of :
def my_funtion_name_that_should_not_be_too_long(self, param1, param2,...):
""comments that say what the function do, the bigs part of
special things in it, when to call it, and warning about when not
using it"""
start of my code, with long lines ;)
The "separation" between the prototype and code is not something I really
like.
> hard to read and understand (e.g.
> "properties" in shinken/host.py),
Yes, this is not simple. That why I wrote a text about it in the official
doc (developers part). I'll add some part of it in the comments too.
> too less comments in complicated parts
> (e.g. Timeperiod.resolve_daterange).
>
Oh you take the less comment part of the code, it's not fair :)
>From ohlho souce, we've got 27% of comment in the code ;) But it's true the
timeperiod part for the resolv is not so comment (it's a regexp function,
and it's something I do quickly because I just HATE regexp). The timeperiods
method that need a lot of comments is the get_next_valid (and invalid), and
in daterange the start_end functions that are quite ok with it.
Do you see any other part of code that need more comments?
>
> Shinken may have good chances to attract python programmers, since it is
> an interesting project. But I strongly suggest converging to the PEP 8
> coding style. Otherwise programmers will be distracted from developing
> shinken.
>
Yes, it's true and it's something very important. In the begining of this
thread, we talk about the pep8, pylint and coverage. I'm just finishing some
bug hunting in the action.py part and the 0.4 will be ok for feature/bugs I
think (and it's "production ready" will be ok). But before the 0.4 is out,
I'm planning a big pylint pass (the previous one was near the 0.1 if I
remember right) and coverage too. I'll try to hunt the 80 lines where it's
better to cut the line, but I'm still wondering how to do in the
"properties" dicts in fact.
I will try to change the # space, but it will take more time, so I don't
think all will be ok for the 0.4 version. By the way, we got now an
integration server (an hudson server) and I'll try to see if we can plot the
pylint/coverage output, so we can follow this code quality.
Thanks for your "bugs", and if you see code lace that need more comments, or
even patches, I take them :)
Jean
>
> --
> Schönen Gruß - Regards
> Hartmut Goebel
> Dipl.-Informatiker (univ.), CISSP, CSSLP
>
> Goebel Consult
> Spezialist für IT-Sicherheit in komplexen Umgebungen
> http://www.goebel-consult.de
>
> Monatliche Kolumne: http://www.cissp-gefluester.de/
> Goebel Consult mit Mitglied bei http://www.7-it.de
>
>
>
>
> ------------------------------------------------------------------------------
> Increase Visibility of Your 3D Game App & Earn a Chance To Win $500!
> Tap into the largest installed PC base & get more eyes on your game by
> optimizing for Intel(R) Graphics Technology. Get started today with the
> Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs.
> http://p.sf.net/sfu/intelisp-dev2dev
> _______________________________________________
> Shinken-devel mailing list
> Shinken-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/shinken-devel
>
>
------------------------------------------------------------------------------
Increase Visibility of Your 3D Game App & Earn a Chance To Win $500!
Tap into the largest installed PC base & get more eyes on your game by
optimizing for Intel(R) Graphics Technology. Get started today with the
Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs.
http://p.sf.net/sfu/intelisp-dev2dev
_______________________________________________
Shinken-devel mailing list
Shinken-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/shinken-devel
