[Geany-devel] Ideas on increasing quality of plugins

[Colomban Wendling]

Le 22/02/2011 22:59, Frank Lanitz a écrit :
>> Le 22/02/2011 17:28, Frank Lanitz a écrit :
>>> Hi folks,
>>>
>>> And again it happen to me that Geany did crash due some issue with a
>>> plugin which might not have been tested very well before checking in
>>> /committing.
>>> However, I don't want to point with my finger to any developer so
>>> I'm asking how we could improve overall quality of plugins inside
>>> release as well as on trunk.
>>>
>>> Failures I was recognizing in past did start with simple case/data
>>> type warnings on compile time up to segfaults caused by not correct
>>> initialized pointers as well a number of memory leaks. I know its
>>> not possible to write 100% clean code as well as I'm aware of my
>>> code isn't very well too. But I really like to change something
>>> here.
>>>
>>> So my 1st suggestion is to remove all plugins which do have known
>>> issues and don't compile with some -W-flags (needs to be defined)
>>> from common build until these are fixed. Also remove plugins which
>>> don't bring propper documentation as well are unmaintained for some
>>> time.
>> Well... I'm puzzled. Disabling unperfect plugins is a good idea in
>> theory for the stability of Geany, but... how would be plugins tested
>> then?
> 
> This is something I like to get clarified with my question ;)
A "tester team"? I doubt we have enough testers for that, but... ^^

>>> Unfortunately this is not a complete list. So maybe we could just
>>> remove every plugin and just start to create a white list.
>> Again, a false-positive of a white-list is that "blacked" plugins
>> won't get real testing.
> 
> This is not necessary true. It would be inside mainstream and would be
> missing this testing, thats correct. But maybe there is some other
> process in place? 
Maybe... which ones? :-'

>>> Ideally (from my point of view) we could set up some kind of review
>>> process but I'm afraid it will not possible due lag of resources -
>>> somebody needs to read and understand the code.
>> Well... I might do basic review on demand. Not necessarily
>> understanding all the code, but reading it as a C developer and
>> reporting what I find wrong. Such a basic review probably needs about
>> 1 or 2 hours, but I personally feel this kind of code review more as
>> "rest time" than "hard work" ;)
> 
> I always like writing code more than try to understand others. Just a
> thought ... Maybe you can do some review on some of the other plugins
> if you have some need for a rest? ;) Would be really cool I guess.
I'd better do this after the plugin's author told she was interested.
Not to loose my spare time perhaps, and not to end up with bad feeling
of somebody feeling offended. As you said: communication!

So, let's directly ask:
Dear plugins authors, are you interested by me reviewing your code and
discussing with you what may be improved/should be fixed/whatever? I'd
be happy to do so if you're interested :)
And if you prefer, I might simply send you a patch with mys suggested
changes/fixes.

>>> Also we might could work
>>> with tests for typical things - somebody only have to code them. I
>>> just don't see any big alternatives at the moment.
>> Like what? I doubt a specific tool would have most benefit over more
>> generic (and already written :p) tools -- I think of a static code
>> checker, maybe cppcheck or clang --check; and the Runtime Error
>> Checking King, I named Valgrind.
> 
> I did have a look onto Valgrind... well.... I started to have a look
> but I wasn't able to figure out how its working in a useful way. Some
> more playing around is needed there I guess. 
Yep, Valgrind is not necessarily an easy-to-use tool, but it's powerful.
In practice, I mostly think about memcheck (a Valgrind's tool -- let's
say it's mode of Valgrind), which helps finding and fixing:
*) invalid memory usage (read or writes outside of bounds)
*) memory leaks

The problem with Valgrind is that it's "too precise" sometimes: it will
report each and every problem it finds in the program, not only "your
code". So, bugs or leaks in other libraries you're not responsible of
will also be shown. To avoid being spammed with such reports, Valgrind
supports "suppression files" that tells him what's not worth reporting.
But someone need to write such a file...

>>> What do you think about?
>> To summarize: a white list is a kinda bad idea IMHO since it'll avoid
>> user testing. However, I think it's a good idea to enforce code
>> quality checking.
> 
> I agree on lot of user testing might will be missing but due some of
> the bugs experienced with geany-plugins 0.20 I assume its not too much
> missing :( Beside of this I totally second your opinion. 
Sad to hear that user faced so much problems. But then, you're probably
right that we should do something about it -- and that we have to find
out what :D

Cheers,
Colomban