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
