D23813: Port away from foreach loops over arguments without calls to owner class

[Friedrich W. H. Kossebau]

kossebau added inline comments.

INLINE COMMENTS

> ahmadsamir wrote in kkeysequencewidget.cpp:127
> I admit "making them const as needed" is badly phrased; I meant whether the container iterated-over is const to begin with, or can be made const in the range-for to avoid a detach/deep-copy ... etc, depending on the code and what it does. The same for the first argument of the range-for loop.
> 
> The "addition"/change I was mainly talking about is the micro-optimization of not calling toString() twice; I think such basic coding practice changes are OK in this context.

Still unsure what you mean with the constness, as the QHash `shortcuts` has been const all the time, and thus the access methods and its returned references.  If you meant the initially proposed `const QKeySequence &seq = it.key();` that was just an alias reference to something const ref before (`it.key()`), whose idea was to not touch the other existing code as well as make it more obvious to the code reader what `it.key()` is. It did not change any constness.

Back to your original comment:
So here my 2 penny collected over decades: avoid that. One change/aspect at a time. No additional clean-up., as basic as it is (even no whitespace changes,  unless line touched anyway).

- The commit message might miss to mention that change, or make it more complicated to read because it lists all the while-at-it changes.
- There are no obvious changes, unless documented. What is clear to the commit creator, might be unclear to the commit reader, as they have another context
- Line-wise commit annotation mark-up will be set for lines which are changed while not relevant for the actual main change (which only would be mentioned in the commit message first line/title)
- One is concentrated on the main change, and might miss important details relevant to that other change, and introduce regressions.

You may discard these 2 penny of mine, but let's talk again in some years ;) Better though ask the search engine for what other people recommend as best commit practice & compare. Still you are free to collect your own experience: if young people only did what old people tell them, new discoveries would never be made ;) But most of the times...

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D23813

To: kossebau, dfaure
Cc: ahmadsamir, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190912/438a89ad/attachment.html>