- User Since
- Feb 10 2018, 7:40 AM (57 w, 2 d)
Sat, Mar 9
Good point, Yuri. We should look into other cases as well.
Thu, Mar 7
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!
Mon, Mar 4
Alex, it looks good now - thanks! Let me test it and I'll get back to you in a few days.
Sat, Mar 2
Mon, Feb 25
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.
Mon, Feb 18
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.
Tested, works as expected. Thanks a lot Toni!
Jul 15 2018
I have trouble applying the patch to current master:
$ arc patch D13499 INFO Base commit is not in local repository; trying to fetch. Created and checked out branch arcpatch-D13499. Checking patch krusader/Panel/panelfunc.cpp... error: while searching for:
Jul 8 2018
Reviewed and tested - works nicely!
Please push this.