mlaurent updated this revision to Diff 80170.
mlaurent added a comment.
Fix David comment
REPOSITORY
R242 Plasma Framework (Library)
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D19913?vs=80169=80170
BRANCH
compile_without_foreach (branched from master)
REVISION DETAIL
mlaurent updated this revision to Diff 80169.
mlaurent added a comment.
Port last element to for(...:...)
REPOSITORY
R242 Plasma Framework (Library)
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D19913?vs=80168=80169
BRANCH
compile_without_foreach (branched from master)
mlaurent updated this revision to Diff 80168.
mlaurent added a comment.
Rebase against master
REPOSITORY
R242 Plasma Framework (Library)
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D19913?vs=55796=80168
BRANCH
compile_without_foreach (branched from master)
REVISION DETAIL
nicolasfella added a comment.
The title says plasma-desktop, but this is plasma-framework?
REPOSITORY
R242 Plasma Framework (Library)
BRANCH
compile_without_foreach (branched from master)
REVISION DETAIL
https://phabricator.kde.org/D19913
To: mlaurent, dfaure
Cc: nicolasfella,
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.
Looks like this got lost/abandoned? It's painful to review (because it's so
long), but it shouldn't be lost work...
Can you rebase and see if it still applies?
Maybe in the future
mlaurent added inline comments.
INLINE COMMENTS
> broulik wrote in coronatest.cpp:216
> why not make the container `const`?
Because this variable is use in code see line 228
REPOSITORY
R242 Plasma Framework (Library)
REVISION DETAIL
https://phabricator.kde.org/D19913
To: mlaurent, dfaure
broulik added inline comments.
INLINE COMMENTS
> coronatest.cpp:150
> +const auto containments = m_corona->containments();
> +for (auto cont : containments) {
> switch (cont->id()) {
Please always annotate `auto` with e.g. asterisk or ampersand depending on type
>
mlaurent updated this revision to Diff 55796.
mlaurent added a comment.
Rename some variables
REPOSITORY
R242 Plasma Framework (Library)
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D19913?vs=54920=55796
BRANCH
compile_without_foreach (branched from master)
REVISION DETAIL
apol added a comment.
etc.
INLINE COMMENTS
> datamodel.cpp:309
>
> -foreach (const QString , m_dataSource->data()->keys()) {
> +const auto lst = m_dataSource->data()->keys();
> +for (const QString : lst) {
same?
Also this should clearly be using iterators, no?
>
mlaurent updated this revision to Diff 54920.
mlaurent added a comment.
Fix comment reported by Apol
REPOSITORY
R242 Plasma Framework (Library)
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D19913?vs=54407=54920
BRANCH
compile_without_foreach (branched from master)
REVISION
apol added a comment.
It's bad enough that we need to give things names because Qt keeps detaching
on foreach, I'd be giving at least more semantic names to these temporary
variables.
INLINE COMMENTS
> coronatest.cpp:149
>
> -foreach (auto cont, m_corona->containments()) {
> +
mlaurent added a comment.
Ping ?:)
REPOSITORY
R242 Plasma Framework (Library)
REVISION DETAIL
https://phabricator.kde.org/D19913
To: mlaurent, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns
mlaurent created this revision.
mlaurent added a reviewer: dfaure.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
mlaurent requested review of this revision.
REVISION SUMMARY
compile without foreach
TEST PLAN
autotest ok
REPOSITORY
R242 Plasma
13 matches
Mail list logo
