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
