D28223: Add "Stat" prefix to StatDetails Enum entries

David Faure noreply at phabricator.kde.org
Tue Mar 24 19:02:13 GMT 2020


dfaure added inline comments.

INLINE COMMENTS

> statjob.cpp:106
>  {
> -    KIO::StatDetails detailsFlag = KIO::StatDetail::Basic;
> +    KIO::StatDetails detailsFlag = KIO::StatDetail::StatBasic;
>      if (details > 0) {

This is a weird way of doing this.

A C-style enum is used like KIO::StatBasic.
Here you're using the C++11-class-enum syntax on a C-style enum, which I'm not sure all compilers accept, and which leads to a redundant "Stat".

I suggest to pick one of those two solutions and stick to it:

1. C-style enum as in this change, but then it's used as KIO::StatBasic etc.

2. "enum class" and then you *don't* need the Stat prefix in the values, i.e. then you can keep writing KIO::StatDetail::Basic.

The current patch makes it look like StatBasic is in the StatDetail "namespace" when in fact it's not.

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, kossebau, dfaure
Cc: davidre, broulik, 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/20200324/86061a38/attachment.html>


More information about the Kde-frameworks-devel mailing list