This revision was automatically updated to reflect the committed changes.
Closed by commit R236:549fc06ffa21: Bulk port away from foreach (authored by
kossebau).
REPOSITORY
R236 KWidgetsAddons
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D23597?vs=65089&id=65614
REVISION DETAIL
h
kossebau added a comment.
Thanks. In any case, would wait for after 5.62 tagging/branching, as by
experience there still might be one or the other regression slipped in with
porting from foreach, so having some weeks of testing from more people running
git master makes me feel better, before
dhaumann accepted this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.
I think this patch is good to go in.
REPOSITORY
R236 KWidgetsAddons
BRANCH
portmostfporeach
REVISION DETAIL
https://phabricator.kde.org/D23597
To: kossebau, #frameworks, cfeck,
kossebau added inline comments.
INLINE COMMENTS
> kossebau wrote in kfontsizeaction.cpp:93
> Will test is this works, but IIRC actions() returned a normal left-side value
> thingie, which qAsConst does not want to take.
Yes, `qAsConst(actions())` does not work, as actions() is a r-value type, w
kossebau updated this revision to Diff 65089.
kossebau added a comment.
- align * & & with var name, not type, by current KF coding style
- fix "fir" for "for"
REPOSITORY
R236 KWidgetsAddons
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D23597?vs=65032&id=65089
BRANCH
portmos
cfeck added inline comments.
INLINE COMMENTS
> kacceleratormanagertest.cpp:35
> +const auto menuActions = menu.actions();
> +for (const QAction* action : menuActions) {
> if (action->isSeparator()) {
Please use KF5 coding style: `Type *var` instead of `Type* var` (also for `&`
kossebau added a comment.
Thanks for review @dhaumann :) Well, if you like I can give KTextEditor a
try, motivated there with my KDevelop hat on :) Let's see when I am sleepless
at the computer next time ;)
INLINE COMMENTS
> dhaumann wrote in fonthelpers.cpp:97
> Optionally, you could even
dhaumann added a comment.
PS: could you do the same for kate.git ? :-D
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D23597
To: kossebau, #frameworks, cfeck
Cc: dhaumann, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
dhaumann added a comment.
I think the patch is fine: +1
Please address/comment on `fir` :) besides that, another review won't hurt,
since you simplify the code in 1-2 places, i.e. the changes are slightly more
than just the transition to `for`.
INLINE COMMENTS
> fonthelpers.cpp:97
>
kossebau created this revision.
kossebau added reviewers: Frameworks, cfeck.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
kossebau requested review of this revision.
REVISION SUMMARY
Few are left as they are with code which needs further analysis
and pos
10 matches
Mail list logo