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
