Category: Code quality
Mentor: bcooksley
Start date: Fri Mar 19, 2010
Duration: 2 weeks
Students: 1-10

This Course will guide you through using the results of KDE's Krazy utility to fix issues in the code which can range from internationaliation to speed optimisations.

By fixing some issues, you will also learn something about common pitfalls which can be avoided in the future. Further, you will be guided in communicating with KDE's reviewboard installation or mailing lists, which are important tools in KDE development.

Krazy2
The tool Krazy Code Checker is a perl script with a collection of code checking plugins. Each day it checks the code in trunk and the results are published at English Breakfast Network.

Requirements

• A system running 4.4.1 or Trunk ( either Distribution packages or Self built is fine ).
• Ability to program in C++ using Qt or KDE libraries.

Content

• Choose a subject on English Breakfast Network, e.g. kdepim/kmail or your favorite application.
• Post back here with your selection so that two items aren't worked on at the same time.
• Try to fix those issues following the Why should I care? info boxes.
• Create a patch and submit it for review using the appropriate workflow.

Installing krazy2
It is useful to use a local krazy2 installation, because the report on English Breakfast Network is created only once a day.

• Follow the guide at Techbase.
• If you have installed it, you can run krazy2all in your source code directory.

Some tips

• Patches can be viewed with the "svn diff" command
• To create a patch file, pipe the output to a file. "svn diff > krazy-fixes.patch"
• Post the patch first here. So we can have a look at it and learn together.
• Create small patches, e.g. one patch for each category.
• If the application has a Review Board component, then use that instead, as it is easier for developers to review patches that way.
• Otherwise, send the patch to the appropriate mailing list, specifying the category and subject in the mail.

Communicating in the forum

• Please inform us about the subject you have chose as a reply to this thread, so that there are no duplicates.
• If you would like to, please post the mail to the mailing list with your patch as a reply to this thread.

Note that this course has begun, those who wish to participate, please reply.

I'll participate. I can run trunk and could start with kdepim/kjots. I'm quite sure I'll have time to do other parts/apps as well.

Just to add to Ben's notes, there's also some useful explanations of some of the different messages and required fixes on the TechBase page. It would be appreciated if you can add to this list as you work out some of the other messages and required fixes.

Thanks!

@lliehu: Welcome to the course. You may start immediately if you wish.

I'll give kdesupport a shot, starting with akonadi I suppose.

It's unclear to me what I'm supposed to do with patches though.

Welcome to the course Glen.

Patches can be posted to this topic ( please pastebin the actual patch ) for review, then they can be put to the developers of the application for final review and committing, either through reviewboard or their mailing list.

I wish to participate....

Welcome to the course ravirdv.

To begin, you should choose a module that interests you ( such as kdeutils for instance ) then begin using Krazy2 to find issues and fix them.

I've created a patch for kdepim/kjots, category qclasses: http://pastebin.com/aat2XUDj

There are 10 issues left, but I couldn't think of any way to solve them. Besides, it's only complaining that QTextEdit is used when referencing either an editor or a browser and it's not known which one it is. QTextEdit is the most "specialized" base class they both have ([KJotsEdit : KRichTextWidget : KRichTextEdit : KTextEdit : QTextEdit] and [KJotsBrowser : KTextBrowser : QTextBrowser : QTextEdit])

kdesupport/akonadi
Check for assignments to QString::null, 3 issues.
http://pastebin.org/120751

kdesupport/akonadi
Check single-char QString operations for efficiency.
http://pastebin.org/120765

I had to tack on a "(const char *)" to get the response.cpp offenders fixed up. I'm not entirely sure that's the right way to go about things.

@lliehu: Patch also looks fine. Since this may affect functionality or behaviour, you should probably post it to Reviewboard. The base directory for your patch is svn://anonsvn.kde.org/home/kde/trunk/kdesupport/

@Glen Kaukola: Patch 1 looks good, this can probably be committed directly. Do you have an svn account?

For patch two, use QChar('+') instead of casting it to a std::string ( which doesn't work on some platforms )

bcooksley wrote:@Glen Kaukola: Patch 1 looks good, this can probably be committed directly. Do you have an svn account?

No, no svn account.

bcooksley wrote:For patch two, use QChar('+') instead of casting it to a std::string ( which doesn't work on some platforms )

Gives me this if I try and go with QChar:
http://pastebin.com/03d8pYv1

Makes sense, as I don't see any QByteArray methods for dealing with QChars. Course of action?

I can commit patch 1 for you if you wish, or you can submit it via reviewboard and the PIM developers can commit it.

Hmm... that is unusual. Other than using C casts instead of C++ casts, though that patch is fine. I would suggest posting that one through reviewboard however.