D25010: [StatJob] Use A QFlag to specify the details returned by StatJob

Méven Car noreply at phabricator.kde.org
Mon Mar 23 08:45:17 GMT 2020


meven marked 2 inline comments as done.
meven added inline comments.

INLINE COMMENTS

> kossebau wrote in global.h:320
> This injects generic terms like `Basic`, `User`, `Time`, `Acl`, etc. into the KIO namespace, with no futher hint that these belong to this very enum, resulting in potential wrong usages (due to completion-based coding when being convertable to int) or in potential conflicts with other future additions.
> 
> Sadly no time to follow the review. Had this been discussed before? Ideally those flags would get more explicit names, like `BasicDetail` (hm, what is basic actually), `UserDetail`, etc.
> 
> Could not find naming recommendations for current Qt, but here some old one, scroll to the section "Naming Enum Types and Values": https://doc.qt.io/archives/qq/qq13-apis.html#theartofnaming

Yes it had been discussed and what you suggest was in a previous iteration : https://phabricator.kde.org/D25010?vs=69181&id=69385&whitespace=ignore-most#change-FL2qoDJhfEB5

Somehow I removed it :
https://phabricator.kde.org/D25010?vs=70051&id=70088&whitespace=ignore-most#change-FL2qoDJhfEB5

But I can't find why I did this, I guess it was a merging/synchronizing issue and lost those changes.
Have I already mentioned how much I dislike arcanist as I work on two machines, it is quite annoying.

Unless @dfaure you find a reason why I shouldn't do it, I will add those prefix back, since KF 5.69 is not yet out of the door, it is possible.

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, dfaure, kossebau
Cc: mlaurent, dalbers, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200323/84580075/attachment.html>


More information about the Kde-frameworks-devel mailing list