- User Since
- Feb 10 2018, 7:40 AM (66 w, 3 d)
Sat, Apr 27
Thanks for the fix and refactoring, Alex! Compiles and works as expected. Please check my inline comments.
Apr 15 2019
Thank you, Yuri and Alex!
Apr 10 2019
Apr 8 2019
QOverload is only available in Qt >= 5.7, and Qt 5.9 is the earliest supported version.
See also https://bugs.kde.org/show_bug.cgi?id=405212 .
I proposed this change in the discussion of D19623 and there were no objections for a month.
Mar 29 2019
I agree that WriteOnly is the best approximation to the NewOnly for older Qt. However, NewOnly is more advanced:
Mar 9 2019
Good point, Yuri. We should look into other cases as well.
Mar 7 2019
Tested with various umasks and local / remote fs - works nicely. Please don't forget to fix the build (and possibly, the commit messages) before merging. Thanks!
Mar 4 2019
Alex, it looks good now - thanks! Let me test it and I'll get back to you in a few days.
Mar 2 2019
Feb 25 2019
Refactoring code is fine, however setting 644 unconditionally introduces a security issue. The permissions for new files should follow umask setting. Otherwise users may unintentionally share their new files with other users.
Feb 18 2019
Nice fix and improvement. Thanks!
Thanks for improving the code quality, Alex!
- fixed build and formatting
Due to no response from Davide, I'm taking over. I also see lots of deprecation warnings and would like to fix it. The most recent revision doesn't compile, however. I have a fix and will update shortly.
I verified it's working correctly in both cases. Thanks for the patch! Please push it to the master, I'll take care of the stable branch afterwards.
Feb 13 2019
The code looks fine to me. Let me test it over the weekend and I'll sign off if no issue found. I can also test a new code path if there is a way to modify v0.25 accordingly.
Nov 1 2018
Oct 27 2018
Also, summary is incorrect:
This is just the fist part, more will follow.
Oct 19 2018
Oct 16 2018
Thanks and welcome aboard Miroslav!
Other than minor things noted inline, looks good! Thanks Alex.
Looks much better, thanks!
Sep 22 2018
Thanks Davide! This looks less cluttered.
More importantly, whenever we make a change after this is merged, we will see the warning and fix the code. We put an effort before v2.7 release to make Krusader warning-free and we should keep it this way. So now the -Wsuggest-override is on the list of warning directives that we track. Thanks Davide (and Toni ;) )!
Sep 15 2018
Why not to update the flags for both debug and release (CMAKE_CXX_FLAGS var)? What's your reasoning? Does it hurt the release build?
Sep 13 2018
Indeed, all warnings of this type are fixed. Thanks Davide!
Sep 12 2018
and reproducible adding -Wsuggest-override or -Winconsistent-missing-override (not sure, haven't tested with clang) to CMAKE_CXX_FLAGS
Should we add this into cmake config file to ensure that these flags are always on?
Aug 23 2018
Aug 16 2018
Aug 12 2018
Aug 5 2018
Apply to stable branch. This will go after changelog / news update commits.
- updated documentation ChangeLog
Jul 31 2018
Jul 29 2018
Thanks Toni! Works nicely.
Please also add FIXED: tag. See https://phabricator.kde.org/w/krusader/#commit-patch-guidelines
Please add fallback logic for the old setting. We can deprecate the fallback code with 2.9+ release.
Jul 24 2018
Jul 18 2018
Thanks! Please use git cherry-pick -x when updating 2.7 branch in the future.
Jul 17 2018
For this kind of change please go through reviews in the future because any change becomes live automatically.