KCalCore Clazy fixes

Authored by winterz on Apr 28 2019, 5:26 PM.



Not all issued reported in 'level1' checks are here, but this is most of them.

notice I also re-wrote the recurrence code in vcalformat... clazy led me there and I didn't like the what I saw so I improved the code.

Test Plan

'make test' on my linux system still works

Diff Detail

R172 KCalendar Core
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
winterz created this revision.Apr 28 2019, 5:26 PM
Restricted Application added a project: KDE PIM. · View Herald TranscriptApr 28 2019, 5:26 PM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
winterz requested review of this revision.Apr 28 2019, 5:26 PM
dvratil requested changes to this revision.Apr 28 2019, 8:50 PM
dvratil added a subscriber: dvratil.
dvratil added inline comments.

Since you are touching this, should it be const auto & for clarity? Also couple more times below in this patch.


Should this be in an else branch? Otherwise bydays will be appended twice if the condition above is true


You could use leftRef() and compare with a QLatin1String() to reduce allocations, since you are touching this code.


This looks identical to the code above, should it be moved into its own function? (not necessarily in this review, but if you feel bored ;-))


Same as above.

This revision now requires changes to proceed.Apr 28 2019, 8:50 PM

I will fix those 'auto' variables. I had planned a clang-tidy round of cleanups, but I can do the auto stuff here.
your other suggestions are also good. I'll handle them.

winterz marked 5 inline comments as done.Apr 29 2019, 10:01 PM
winterz added inline comments.

I'm not seeing what you're seeing. I'll revert the logic back


too much work at this time. huge amounts of code in the file really needs refactoring. I'll skip it.

winterz updated this revision to Diff 57221.Apr 29 2019, 10:12 PM
winterz marked 2 inline comments as done.

Dan's comments and suggestions

ach added a subscriber: ach.Apr 30 2019, 2:18 PM
ach added inline comments.

QLatin1String() suggestion not applied.

winterz updated this revision to Diff 57287.Apr 30 2019, 7:28 PM

missing changes for QLatin1String()
thanks for noticing Achim

dvratil accepted this revision.May 4 2019, 7:45 AM

Sorry for the delay. Thanks for the improvements!

This revision is now accepted and ready to land.May 4 2019, 7:45 AM
This revision was automatically updated to reflect the committed changes.