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

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> kossebau wrote in kkeysequencewidget.cpp:127
> 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...

I do try to keep commits atomic, and detail my changes in the commit message. (And no, I won't discard your 2 pennies, I am too poor, experience-wise, to afford that). What I should have done was read the code starting at the top, where shortcuts is declared const. Anyway thanks for explaining things, that's always appreciated. :)

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/8879a78b/attachment.html>