Re: [Shinken-devel] code quality

[nap <nap...@gm...>]

Hi,

On Sun, Nov 28, 2010 at 11:15 PM, Hartmut Goebel <h.g...@go...
> wrote:

>  Am 25.11.2010 09:52, schrieb nap:
>
>
>  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.
>
> Esp. for the properties I do not agree. In some modules, these are lines
> and lines of unstructured text. Hard to read, hard to what one is searching
> for.
>
what do you mean by unstructured text? For properties?



>
>  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 :(
>
> In my experience this most often is a sign that the code should be
> rewritten. So you may want to have another look at these places :-)
>
No it's a sign of beter choose of properties name, but I cannot change them
:)
There is not such case where we've got a lot of nested code, so it's really
a problem of variable names for a lot of the code. But I'll cut the "if
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 :
>
> Well, but shinken is a Python program :-) Even if you personally dislike
> docstrings, you should rethink this. Python programmers love dostrings and
> using them will be some kind of marketing ;-)
>
Yes it's true. But it will make C coders hate even more the code, and most
of monitoring guys are from C in the "Nagios world" ;)
But let try the docstrings. After all, we can also try a tool like Sphinx
and some people will like to use it, and so will begin to hack the code. And
if that make Python guys look at the code, it's good after all.


>
>
>   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 :)
>
> [...]
>
>  it's a regexp function, and it's something I do quickly because I just
> HATE regexp).
>
> Most people I know hate regexp (except for Perl hackers maybe),
>
<joke>but perl hackers are not humans :) </joke>

>  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.
>
> Just like this:
>     properties = {
>         'command_name': {
>             'required': True,
>             'fill_brok': ['full_status']},
>         'command_line':
>             {'required': True,
>              'fill_brok': ['full_status']},
>         'poller_tag': {
>             'required': False, 'default': None},
>         }
>
Hum... seems quite good it's true. I feared that it should be hard to find
the property name when you ened to see/change a property property. But here
we see it quite good in fact. The code will be quite long after that, but
it's another problem.


>
>  Thanks for your "bugs", and if you see code lace that need more comments,
> or even patches, I take them :)
>
> Just have a look at trak :-)
>
Thanks for you proposals and help on code quality. We put the Hudson with
the pylint (I don't succeed from now for the coverage pass for Hudson).

It's available at : http://shinken-monitoring.de:8080/

I'm on the pylint pass. It will take some days, but the pylint curve will
fall in more acceptable values :) (Hopefuly, there are few huge problems,
only medium ones).


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
> Shi...@li...
> https://lists.sourceforge.net/lists/listinfo/shinken-devel
>
>