Requesting freeze exception for JtG

[David Faure]

On Wednesday 23 November 2011 00:28:07 Pau Garcia i Quiles wrote:
> > 1) Changes to ui_standards.rc
> > 
> > This is a high-risk change. It affects every application that uses
> > XMLGUI and KParts plugins. I did quite a lot of work in this area in
> > KDE 3.x and I know that changes to this file could have a lot of
> > knock-on effects regarding gui merging at the time. I don't know if
> > this is still the case, but it's a big concern IMHO.
> 
> Now that we have established that there might be a risk (I took care
> of not breaking anything, but you can never double-check enough),
> could you please check the patch does not break anything? I'd like to
> move forward as soon as possible so that the strings can be
> translated.

In general, I agree that ui_standards.rc is a touchy place. On the other hand, 
the help menu is typically "all or nothing" in the apps, and adding an action 
should be fine (less potential changes than adding a group, or a merge point, 
or moving stuff around).

But why is the action added to KStandardAction? The only user (in terms of 
code) is KHelpMenu, isn't it? Why not just define and create the action there?
Adding API for this seems overkill. Imagine we want to cancel (or rename) that 
program at some point, surely we don't want source compatibility to prevent us 
to do that.

I see that you did that "to mimic what was already there", but it's not 
useful. Just have a KAction* createJoinTheGameAction() in khelpmenu and call 
that from both the "add to kactioncollection" code path and the menu() method.

khelpmenu seems to have been ported from qt3 without much thinking, the 
actions in menu() could just use kstandardaction too, instead of duplicating 
the action details... but that's for frameworks 5 :)

The addition to KStandardShortcut is even less useful, without a default 
shortcut. Please remove that altogether.

khelpmenu.cpp: weird indentation, with statements starting at column 0.

All this is while reviewing this commit:
http://quickgit.kde.org/?p=clones%2Fkdelibs%2Fpgquiles%2Fkdelibs-
jointhegame.git&a=commitdiff&h=5f50f16480667b52c38b8e53c0bb33888bd50309

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Sponsored by Nokia to work on KDE, incl. KDE Frameworks 5