ervin requested changes to this revision.
ervin added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> kconfigcompiler_test.cpp:80
> "test_signal",
> + "test_notifiers",
> "test_translation_kde",
This seems unrelated... So we had this test case available but it was in fact
never run?
> test_subgroups.kcfg:10
> + </kcfgfile>
> + <group name="$(SubGroup)" parentGroup="$(GeneralGroup)">
> + <entry name="Foo" type="Bool">
Now that I see it, I think I'd go for "parentGroupName" since this is not
referential and really about the name (like the name parameter)
> test_subgroups.kcfg:20
> + </group>
> +</kcfg>
Probably worth also having cases with:
- the group name being parameterized but not the parentGroup
- both name and parentGroup not be parameterized
Just to have all the possible combinations.
> KConfigSourceGenerator.cpp:354
>
> +// TODO : Some compiler option won't work or generate bogus settings file.
> +// * Does not manage properly Notifiers=true kcfgc option for parameterized
> entries :
Unrelated right, this is not due to your patch, or am I confused?
> KConfigXmlParser.cpp:502
>
> + QString parentGroup = e.attribute(QStringLiteral("parentGroup"));
> +
I'd const auto it, or at least const.
REPOSITORY
R237 KConfig
REVISION DETAIL
https://phabricator.kde.org/D27133
To: crossi, ervin, dfaure, #frameworks
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham,
bruns
