dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> slavebase.cpp:1525
> +
> +#ifndef KIOCORE_NO_DEPRECATED
> +PrivilegeOperationStatus SlaveBase::requestPrivilegeOperation()
`#if KIOCORE_BUILD_DEPRECATED_SINCE(5, 65) `
(notice BUILD, not ENABLE)
> slavebase.h:960
> +
> +#ifndef KIOCORE_NO_DEPRECATED
> + KIOCORE_DEPRECATED PrivilegeOperationStatus requestPrivilegeOperation();
The modern way is `#if KIOCORE_ENABLE_DEPRECATED_SINCE(5, 65)`
> slavebase.h:961
> +#ifndef KIOCORE_NO_DEPRECATED
> + KIOCORE_DEPRECATED PrivilegeOperationStatus requestPrivilegeOperation();
> +#endif
`KIOCORE_DEPRECATED_VERSION(5, 65, "Pass QString action to
requestPrivilegeOperation")`
> slaveinterface.h:89
> MSG_PRIVILEGE_EXEC,
> - MSG_SLAVE_STATUS_V2
> + MSG_SLAVE_STATUS_V2,
> // add new ones here once a release is done, to avoid breaking binary
> compatibility
unrelated/unneeded anymore
> slaveinterface_p.h:56
> + // Since 5.65 this is used for sending privilege operation details.
> + // KF6 TODO remove this hack.
> + MetaData privilegeConfMetaData;
Please explain how, for when the time comes (in case you're not around to do
it).
> file_unix.cpp:84
> + QString action, detail;
> + switch(actionType) {
> + case CHMOD:
missing space after `switch` (same rule as `if`, `for` etc.)
> dfaure wrote in file_unix.cpp:81
> `const QVariantList &`
marked as done, but not done
> jobuidelegate.cpp:375
> + const QString details =
> metaData.value(QStringLiteral("privilege_conf_details"));
> + result = KMessageBox::warningContinueCancelDetailed(
> + window(), text, caption, KStandardGuiItem::cont(),
> KStandardGuiItem::cancel(), dontAskAgainName,
two spaces after `=`, remove one
> jobuidelegate.cpp:377
> + window(), text, caption, KStandardGuiItem::cont(),
> KStandardGuiItem::cancel(), dontAskAgainName,
> + KMessageBox::Options(KMessageBox::Dangerous |
> KMessageBox::WindowModal), details);
> + break;
I don't think you need the `KMessageBox::Options(` around the value?
> jobuidelegate.h:155
> const QString &dontAskAgainName = QString(),
> - const KIO::MetaData &sslMetaData =
> KIO::MetaData()) override;
> + const KIO::MetaData &metaData = KIO::MetaData())
> override; // KF6 TODO Add proper API
>
what would be the proper API you have in mind?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D21783
To: chinmoyr, #vdg, #frameworks, dfaure
Cc: mreeves, ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns
