It can use the original KIO URLs. No need for using temporary local files.
And remove the obsolete space character check for file paths for KDiff3.
CCBUG: 401064
pino | |
asensi | |
gengisdave |
Krusader |
It can use the original KIO URLs. No need for using temporary local files.
And remove the obsolete space character check for file paths for KDiff3.
CCBUG: 401064
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
krusader/krslots.cpp | ||
---|---|---|
218–219 |
eh, of course.
See the Git comment/Differential summary. It is not necessary anymore. | |
228–231 | Although this is unrelated, these are safe changes nearby which make the code a bit cleaner. -> "Boy Scout rule" |
Mostly OK from my POV, just please remove the unrelated changes to the patch (i.e. the brackets additions in two places).
I have no idea what "Boy Scout rule" is supposed to mean, however adding unrelated changes makes history reading harder, especially when wanting to check why certain changes were done. Again, material for a different patch than this.
Meanwhile the other issues are talked about, I performed some tests under Kubuntu 18.04 LTS and Kubuntu 19.10 and the resulting code worked. Thanks, Alex, Yuri and Pino!
I removed the two added brackets again.
I will separate those code cleanup changes into different commits in the future.
But having different code reviews for total minor changes like this is unpractical. I have time for Krusader only every few weeks and this is too much overhead. The better choice then is to not do them anymore at all.
In the meantime, the not directly related changes were applied in https://phabricator.kde.org/D27609.
Dear developers,
As code reviews in Phabricator are not going to be migrated to Gitlab (https://marc.info/?l=kde-core-devel&m=158909698416266&w=2), the commented issues were solved and this review is accepted since a long time ago: I'm going to do the git push if there is no objection. Thanks for all the developing and testing!
Taking into account the latest messages here, and after several months of using this helpful commit, I've published the commit before the Phabricator to Gitlab migration started. Please, feel free to suggest or improve whatever you consider.