User Details
- User Since
- Feb 10 2018, 7:40 AM (318 w, 3 d)
- Availability
- Available
Jun 28 2020
May 30 2020
Thanks Toni, I missed that.
May 28 2020
@gengisdave , I don't see this merged. Forgotten?
@asensi , would you like to rebase the change in your fork and create a pull request on GitLab? My VM is working now and it's a useful change - I could help to review and test it.
May 24 2020
Dec 30 2019
This is a right thing to do (connect before calling the emitting method). I just have a request to simplify the comment - something like "don't rely on return value of openUrl as the call is async in general". Thanks!
Dec 12 2019
Sep 16 2019
3rd opinion. I agree it should be shown only if the file is actually going to be hidden (Show Hidden is off in settings), otherwise Krusader looks dumb. I agree that users should be able to disable the warning, because for us it may look like an uncommon operation, however some users may work with dot files extensively. This kind of attention to small details make a huge difference in user experience.
Sep 15 2019
Works great and the code looks clean and neat! Thanks for working on this.
Sep 12 2019
This diff contains combined changes on two separate repositories. Phabricator will have a problem with this kind of change. Please split into two reviews. I'm ok with the content.
Sep 6 2019
Reviewed and tested (I'm using Plasma) - works good. Thanks for the fix, Alex!
Reviewed and tested - works good. Thanks for the fix, Toni!
Sep 3 2019
The first screenshot looks better than others, IMO.
It can be restored from git history in case it's needed. It's your call.
Sep 2 2019
I also have seen it when I tested published changelog files and wanted to remove it. Thanks for bringing my thoughts to live. :)
Reviewed and tested - all good! Thanks for the fix, Toni!
Aug 25 2019
- updated SHA256 checksum
Aug 22 2019
Please always open a review! Committed and pushed changes are deployed directly to live site. People make mistakes, it's normal, and reviews reduce the likelihood of a mistake.
Thanks everyone!
Aug 19 2019
For example, you can fork krusader on github or other platform [...]. What do you think about this idea, Toni?
I don't think people would find it, Nikita.
Aug 18 2019
When I see the usage of the method, like
const float fontWidth = (fm.QFONTMETRICS_WIDTH("WWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWW") - fm.QFONTMETRICS_WIDTH("W")) / 99.0;
or
headerView->resizeSection(KrViewProperties::Ext, QFontMetrics(_viewFont).QFONTMETRICS_WIDTH("tar.bz2 ")); headerView->resizeSection(KrViewProperties::KrPermissions, QFontMetrics(_viewFont).QFONTMETRICS_WIDTH("rwx ")); headerView->resizeSection(KrViewProperties::Size, QFontMetrics(_viewFont).QFONTMETRICS_WIDTH("9") * 10);
which are all dirty hacks, I doubt we need to care too much about the differences mentioned. This code needs to be refactored properly but it's not a goal of this change. QFONTMETRICS_WIDTH will actually become a good marker of the places that need review if someone will find time and courage to make it right one day...
This is a very serious change because now #ifndef Q_OS_WIN or #else block after #ifdef Q_OS_WIN will include the code that previously was ignored. We must look at all those code paths and test features it affects. BTW, does it mean that some of those code paths do not work correctly in the current state?
Good catch, Toni! No objections for the new shortcut. Please look into
- Making the commit headline shorter. Something like "Changed shortcut for the embedded terminal emulator". It's easier to read in the graph view. Please keep everything else, it's very good to have a detailed explanation in the commit message!
- Specifying CHANGED: tag, so this is not missed in the ChangeLog on the next release.
I have no problem when someone is trying to improve a dead branch. If the person finds this useful and wants to share their changes to others — he or she is welcome to do so. It's an Open Source after all. In the same time, I understand what Luigi is saying that it might send a wrong signal to users. In addition, commits we push to the official repo are reviewed and tested, and should be only pushed when approved by at least another dev and no objections from others (unfortunately, it's not enforced due to a weak infrastructure). It means someone needs to test the changes you propose in 10 code reviews. Personally, I have no interest in kde4 commits anymore and even have no environment to test it. I doubt you'll find another dev who has.
Related review: D23197 (same changelog)
Aug 17 2019
Hi Toni,
Aug 16 2019
Aug 9 2019
Updated AppStream files. Thanks Luigi and Toni!
- Updated AppStream files with previous and upcoming release info
Aug 7 2019
@yurchor, this was fast! Thanks!
Please review by August 10, EOD. Thanks!
Jul 31 2019
It's a useful feature for people that like to keep their environment clean. Thanks for working on this, Toni!
If you'd like this fix to be included in v2.7.2, please push it to master by Aug 4 and I'll backport to stable. Please don't forget to specify BUG, FIXED, Differential Revision tags in the commit message.
If you'd like this fix to be included in v2.7.2, please push it to master by Aug 4 and I'll backport to stable. Please don't forget to specify BUG, FIXED, Differential Revision tags in the commit message.
Jul 28 2019
It works properly now - thanks!
I checked the commit you mentioned in the bug. Before it the checkbox was still ignored but the links were always not followed.
It works and I'm fine with the proposed fix. Thanks Toni!