[plasma-desktop] make it compiles without foreach
Needs ReviewPublic

Authored by mlaurent on Mar 20 2019, 12:58 PM.

Details

Reviewers
dfaure
Summary

compile without foreach

Test Plan

autotest ok

Diff Detail

Repository
R242 Plasma Framework (Library)
Branch
compile_without_foreach (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9899
Build 9917: arc lint + arc unit
mlaurent created this revision.Mar 20 2019, 12:58 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptMar 20 2019, 12:58 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
mlaurent requested review of this revision.Mar 20 2019, 12:58 PM
apol added a subscriber: apol.Mar 26 2019, 2:46 PM

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.

autotests/coronatest.cpp
149

maybe call it containments? naming variables with a typename is bad (we even are using auto to not mention it's a list!)

src/declarativeimports/core/datamodel.cpp
401

It already is const.

src/plasma/pluginloader.cpp
567

shouldn't it be const?

mlaurent updated this revision to Diff 54920.Mar 27 2019, 7:02 AM

Fix comment reported by Apol

apol added a comment.Mar 27 2019, 11:59 AM

etc.

src/declarativeimports/core/datamodel.cpp
309–310

same?
Also this should clearly be using iterators, no?

src/declarativeimports/core/tooltipdialog.cpp
59–60

same?
If we have to open an incidence for every comment this gets exhausting.

src/plasma/containment.cpp
299–300

just applets?

src/plasma/corona.cpp
132–133

applets?

mlaurent updated this revision to Diff 55796.Apr 9 2019, 6:04 AM

Rename some variables

broulik added a subscriber: broulik.Apr 9 2019, 6:41 AM
broulik added inline comments.
autotests/coronatest.cpp
150

Please always annotate auto with e.g. asterisk or ampersand depending on type

216–217

why not make the container const?

src/declarativeimports/calendar/daysmodel.cpp
171

While at it const QDate &

src/declarativeimports/core/datamodel.cpp
309–310

Yes, please change it to iterators

mlaurent added inline comments.Apr 9 2019, 7:10 AM
autotests/coronatest.cpp
216–217

Because this variable is use in code see line 228