Hi,
On Tue, 22 Feb 2011 19:36:51 +0100 Colomban Wendling lists.ban@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