ervin requested changes to this revision.
ervin added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> %{APPNAMELC}settings.kcfg:8
> + <group name="ExampleConfigGroup">
> + <entry name="exampleSetting" type="String">
> + <label>The Exam</label>
Now that I think about it, what about using both key and name here? To have a
nice property name and a "stranger" storage key?
Most people seem to miss that feature somehow, that'd make it more obvious.
> Messages.sh:3
> +$EXTRACTRC `find . -name "*.ui"` >> rc.cpp || exit 11
> +$XGETTEXT `find . -name "*.cpp"` -o $podir/kcm5_%{APPNAMELC}.pot
> +rm -f rc.cpp
I'm talking mostly out of ignorance here, but shouldn't that harvest also the
qml files somehow?
> kcm.cpp:30
> +
> +%{APPNAME}::%{APPNAME}(QObject* parent, const QVariantList& args)
> + : KQuickAddons::ManagedConfigModule(parent, args)
Ditto
> kcm.cpp:36
> +
> + KAboutData* about = new KAboutData(
> + QStringLiteral("kcm_%{APPNAMELC}"), i18n("%{APPNAME} Configuration
> Module"),
Ditto
Beside that could use auto
> kcm.h:32
> +public:
> + %{APPNAME}(QObject* parent, const QVariantList& args);
> + virtual ~%{APPNAME}() override;
Nitpick: space should be before * and & not after.
REPOSITORY
R242 Plasma Framework (Library)
REVISION DETAIL
https://phabricator.kde.org/D25149
To: tcanabrava, #plasma, #frameworks, mart, ervin
Cc: #plasma, GB_2, yurchor, davidedmundson, ognarb, ervin,
kde-frameworks-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas,
ragreen, michaelh, ZrenBot, ngraham, bruns, alexeymin, himcesjf, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
