[core/*] replace foreach, deprecated, with range-for
AbandonedPublic

Authored by ahmadsamir on Dec 2 2019, 2:58 PM.

Details

Reviewers
aacid
Summary

The code compiles and okular seems to load and work as before, all unit
tests pass except (parttest and epubgeneratortest, but they fail on master
too).

Diff Detail

Repository
R223 Okular
Branch
l-foreach (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19508
Build 19526: arc lint + arc unit
ahmadsamir created this revision.Dec 2 2019, 2:58 PM
Restricted Application added a project: Okular. · View Herald TranscriptDec 2 2019, 2:58 PM
ahmadsamir requested review of this revision.Dec 2 2019, 2:58 PM
aacid added a comment.Dec 3 2019, 11:22 PM

Thanks for the patch!

Some small comments

core/annotations.cpp
2094

i'm going to commit a patch that makes distanceSqr be const so you won't need this anymore.

2309

same

core/document.cpp
3920

don't do const & for basic types, it's cheaper to copy them than to reference them

4034

same

core/form.cpp
258

same

core/page.cpp
504

we don't need qAsConst here, do we?

core/textpage.cpp
301

if the function is const, m_region_blabla is const already

ahmadsamir updated this revision to Diff 70874.Dec 4 2019, 5:47 AM
ahmadsamir marked 7 inline comments as done.

::distanceSqr() is going to be made const, so qAsConst isn't needed
It's cheaper to copy basic types than reference them
qAsConst isn't needed when iterating over member variable containers in const methods

ahmadsamir added inline comments.Dec 4 2019, 5:49 AM
core/document.cpp
3920

Thanks. Noted.

aacid added a comment.Dec 4 2019, 10:30 PM

Please rebase your patch on current master, since it will fail to merge properly, i prefer you do the work and not me ^_^

Please rebase your patch on current master, since it will fail to merge properly, i prefer you do the work and not me ^_^

FWIW, I always 'git pull --rebase' on the branch right before I 'arc land' :)

aacid accepted this revision.Dec 8 2019, 10:27 PM
This revision is now accepted and ready to land.Dec 8 2019, 10:27 PM

Trying to land this diff I get these errors:

remote: FATAL: W refs/heads/master okular ahmadsamir DENIED by refs/.*
remote: error: hook declined to update refs/heads/master
To git.kde.org:okular
 ! [remote rejected]     c63256c8d18f7f2798019554d62e2c0fb8d0110c -> master (hook declined)
error: failed to push some refs to 'git@git.kde.org:okular'
Usage Exception: Push failed! Fix the error and run "arc land" again.
ahmadsamir abandoned this revision.Dec 9 2019, 11:57 AM

Created a pull request on invent.kde.org. https://invent.kde.org/kde/okular/merge_requests/73