D11012: Add Trash (empty, isEmpty, emptinessChanged)

Roman Inflianskas noreply at phabricator.kde.org
Sat Mar 10 19:24:02 GMT 2018


rominf added a comment.


  In D11012#222653 <https://phabricator.kde.org/D11012#222653>, @elvisangelaccio wrote:
  
  > First of all, please read https://community.kde.org/Policies/Commit_Policy#Always_add_descriptive_log_messages
  
  
  Done. Understood.
  
  > At a first glance, I only see code moved around and I don't understand where the actual bugfix is.
  >  The commit message should describe what the problem is and how the commit is going to fix it.
  
  OK. This patch had 2 aims, that is to refactor the trash code and fix the bug. I split the patch and explicitly stated the purpose of this patch (refactoring). Is this OK now?

INLINE COMMENTS

> markg wrote in dolphintrash.cpp:30-31
> Now you still need to fix the indentation ;)
> Sorry, nitpicking...
> 
> the semicolon should be on the line below and indentation should be 4 spaces.

OK, but... It's the same as in files "informationpanelcontent.cpp", "filemetadataconfigurationdialog.cpp", "trashsettingspage.cpp", etc.

Is it worth to make semicolon consistent in another patch?

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D11012

To: rominf, elvisangelaccio, markg, ngraham
Cc: ngraham, markg, rkflx, elvisangelaccio, #dolphin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180310/32a7db86/attachment.htm>


More information about the kfm-devel mailing list