[Geany-devel] Ideas on increasing quality of plugins

[Colomban Wendling]

Hi,

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?

However, I agree with making official builds with strong error checking
enabled (at least -Werror-implicit-function-declaration comes directly
to my mind). This is static checking that will show most obvious
problems, and hopefully it'll encourage their maintainer to fix them.
However, finding the right flags will not be easy... for example not
allowing any implicit cast may have a false-positive effect where
unexperimented developers might hide a part of those with a cast where a
proper fix would be better.
And even I (yes, I think I'm pretty experimented and knowledgeable about
C) sometimes don't correct a hard-to-fix integer cast warning because I
know this particular implicit cast isn't really important but I also
know it should better be fixed than simply hiding it with an explicit
cast. So I leave the warning, kinda as a "rememberer".

> 
> As a first shoot this could be from my point of view
> 
> - Updatechecker (crash on Windows build)
It then might be disabled only for Windows if it's safe on Unices?

> - GeanyLUA (lot of warnings on compiling time)
> - Pretty Printer (lot of warnings on compiling time)
Why not. Hopefully their author will fix them :)

> - GeanyGDB (nearly unmaintained)
For this one, it also has a candidate for replacement, hasn't it?

> - GeanyVC (de facto unmaintained)
...but AFAICT it's stable. Unmaintained software isn't good, but I doubt
dropping such a feature would please all users.

> - WebHelper (Crash mentioned on SF)
This is a good (I think :D) example of what I talked about above: user
testing. I use this plugin, and at least load it often (heh, I wrote
it...) and never found a crash I didn't fixed. So without testing, I
would probably never have been informed of such a crash, so I'd never
had a chance to fix it.

> 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.

> 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" ;)

> 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.

Finally, I don't point my finger to anybody neither, but I know some of
the developers aren't experienced C developers. They then probably
cannot really review their own code for C problems. And anyway, an
external review is IMHO always a good thing, even for the most
experienced of us: at least it's an opportunity to justify (to
ourselves) how the code is written.

> 
> 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.


Regards,
Colomban