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

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Now that is a quick turn-around, +1 for doing the work :)
  
  No time myself to look at this closely the next days, also not that much into KIO, but here some quick feedback with an API police hat on.
  
  Would be also good to have some patches which make use of the new API, so one could better judge the usefulness.

INLINE COMMENTS

> statjob.h:50
>  
> +    enum Detail {
> +        /// Filename, access, type, size, linkdest

Is there other KIO API which has similar enums, where some consistency would be good to have?

On a first look, the names seems very short & generic to me, having some other name element might avoid semantic collisions perhaps. No idea yet, not looked further.

> statjob.h:50
>  
> +    enum Detail {
> +        /// Filename, access, type, size, linkdest

Please add @since comment

> statjob.h:91
> +     * Selects the level of @p details we want.
> +     */
> +    void setDetails(StatJob::Details details);

Please add @since

> statjob.h:103
>       */
>      void setDetails(short int details);
>  

Not deprecated now?

> statjob.h:174
> +
> +constexpr static StatJob::Details DefaultDetails = StatJob::Detail::Basic | StatJob::Detail::User | StatJob::Detail::Time | StatJob::Detail::Acl | StatJob::Detail::ResolveSymlink;
> +

Does KIO::DefaultDetails make sense beyond stat in the KIO namespace? How does it match other similar flags?
Perhaps should be more bound to "stat" by the name.
Please also add documentation, incl. "@since"

> statjob.h:206
> + * @return the job handling the operation.
> + */
> +KIOCORE_EXPORT StatJob *stat(const QUrl &url, KIO::StatJob::StatSide side,

Please add @since

> statjob.h:209
> +                             StatJob::Details details, JobFlags flags = DefaultFlags);
>  /**
>   * Find all details for one file or directory.

Please wrap the deprecated API call (incl. API dox comment) with visibilty controlling #if/#endif., so here

  #if KIOCORE_ENABLE_DEPRECATED_SINCE(5, 64)

> statjob.h:234
>   * @param flags Can be HideProgressInfo here
>   * @return the job handling the operation.
>   */

Please add "@deprecated Since 5.64. Use KIO::stat(const QUrl &, KIO::StatJob::StatSide, StatJob::Details int, JobFlags)"

For all the recommended details to add when deprecating API (also with the new deprecation macros), please see
https://community.kde.org/Policies/Library_Code_Policy#Deprecation_of_API

> statjob.h:239
>                               short int details, JobFlags flags = DefaultFlags);
>  
>  #if KIOCORE_ENABLE_DEPRECATED_SINCE(4, 0)

#endif

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, dfaure, kossebau
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20191029/58ee5fda/attachment-0001.html>