[Geany-devel] Ideas on increasing quality of plugins

Tue Feb 22 21:59:49 UTC 2011

Hi, 

On Tue, 22 Feb 2011 19:36:51 +0100
Colomban Wendling <lists.ban at herbesfolles.org> wrote:

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

This is something I like to get clarified with my question ;)

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

I agree. 

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

So... you know your task ;)

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

Yes. Valid point. 

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

Nick did take it over as the original developer didn't maintain it
anymore. There have been only the most important things done IIRC due
of this point.

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

Yepp. But this would also include a 100% review uf the new plugin and
check whether it really does its job. At least for me I cannot decide. 

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

There are some issue with the plugin. Of course a huge number of poeple
would miss it. 

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

Yes, thats true. Without good testing you cannot find all bugs. 

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

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

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

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

I agree. 

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

Cheers, 
Frank
-- 
http://frank.uvena.de/en/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.geany.org/pipermail/devel/attachments/20110222/e3728b8a/attachment.pgp>