Reply to topic

Krazy fixes 2

User avatar bcooksley
Administrator
Posts
19763
Karma
87
OS

Krazy fixes 2

Fri Mar 19, 2010 9:05 am
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.


KDE Sysadmin
[img]http://forum.kde.org/content/bcooksley_sig.png[/img]
User avatar bcooksley
Administrator
Posts
19763
Karma
87
OS

Re: Krazy fixes 2

Sat Mar 20, 2010 7:25 am
Note that this course has begun, those who wish to participate, please reply.


KDE Sysadmin
[img]http://forum.kde.org/content/bcooksley_sig.png[/img]
lliehu
Alumni
Posts
9
Karma
0
OS

Re: Krazy fixes 2

Sat Mar 20, 2010 1:51 pm
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.
odysseus
KDE Developer
Posts
3
Karma
0
OS

Re: Krazy fixes 2

Sat Mar 20, 2010 6:58 pm
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!
User avatar bcooksley
Administrator
Posts
19763
Karma
87
OS

Re: Krazy fixes 2

Sat Mar 20, 2010 7:47 pm
@lliehu: Welcome to the course. You may start immediately if you wish.


KDE Sysadmin
[img]http://forum.kde.org/content/bcooksley_sig.png[/img]
Glen Kaukola
Alumni
Posts
39
Karma
0
OS

Re: Krazy fixes 2

Mon Mar 22, 2010 5:26 am
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.
User avatar bcooksley
Administrator
Posts
19763
Karma
87
OS

Re: Krazy fixes 2

Mon Mar 22, 2010 5:50 am
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.


KDE Sysadmin
[img]http://forum.kde.org/content/bcooksley_sig.png[/img]
ravirdv
Registered Member
Posts
6
Karma
0

Re: Krazy fixes 2

Mon Mar 22, 2010 4:54 pm
I wish to participate....


ravirdv, proud to be a member of KDE forums since 2008-Nov.
User avatar bcooksley
Administrator
Posts
19763
Karma
87
OS

Re: Krazy fixes 2

Mon Mar 22, 2010 7:04 pm
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.


KDE Sysadmin
[img]http://forum.kde.org/content/bcooksley_sig.png[/img]
lliehu
Alumni
Posts
9
Karma
0
OS

Re: Krazy fixes 2

Tue Mar 23, 2010 2:35 am
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])
Glen Kaukola
Alumni
Posts
39
Karma
0
OS

Re: Krazy fixes 2

Tue Mar 23, 2010 3:05 am
kdesupport/akonadi
Check for assignments to QString::null, 3 issues.
http://pastebin.org/120751
Glen Kaukola
Alumni
Posts
39
Karma
0
OS

Re: Krazy fixes 2

Tue Mar 23, 2010 4:04 am
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.
User avatar bcooksley
Administrator
Posts
19763
Karma
87
OS

Re: Krazy fixes 2

Tue Mar 23, 2010 4:09 am
@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 )


KDE Sysadmin
[img]http://forum.kde.org/content/bcooksley_sig.png[/img]
Glen Kaukola
Alumni
Posts
39
Karma
0
OS

Re: Krazy fixes 2

Tue Mar 23, 2010 4:17 am
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?
User avatar bcooksley
Administrator
Posts
19763
Karma
87
OS

Re: Krazy fixes 2

Tue Mar 23, 2010 4:48 am
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.


KDE Sysadmin
[img]http://forum.kde.org/content/bcooksley_sig.png[/img]

 
Reply to topic

Bookmarks



Who is online

Registered users: acheshirov, Baidu [Spider], bcooksley, benjaminl, Bing [Bot], Google [Bot], gui-m, hoxtonhopper, kakosf, kamathraghavendra, kde-jriddell, Mamarok, P3lor, Snudl, Yahoo [Bot], zwankfr