sitter requested changes to this revision.
sitter added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> Polkit1Backend.cpp:4
> * Copyright (C) 2009 Radek Novacek <rnova...@redhat.com>
> * Copyright (C) 2009-2010 Dario Freddi <d...@kde.org>
> *
According to the diff of the diff you seem to have lost David's copyright
> Polkit1Backend.cpp:75
> connect(PolkitQt1::Authority::instance(),
> SIGNAL(enumerateActionsFinished(PolkitQt1::ActionDescription::List)),
> this,
> SLOT(updateCachedActions(PolkitQt1::ActionDescription::List)));
>
Is there a reason you use stringy connection syntax instead of &member syntax?
> Polkit1Backend.cpp:178
>
> -bool Polkit1Backend::isCallerAuthorized(const QString &action, QByteArray
> callerID)
> +bool Polkit1Backend::isCallerAuthorized(const QString& action, const
> QByteArray &callerID, const QVariantMap& details)
> {
action has & on the wrong size of the space, same for details ;)
> Polkit1Backend.cpp:239
> // Wait max 2 seconds
> QEventLoop e;
> QTimer::singleShot(200, &e, SLOT(quit()));
Mhhhh. Mhhhhhhhh. I don't really have a suggestion here, but this is an
incredibly dangerous change. Nested event loops can cause all sorts of negative
effects. That's why the isValid had a note in the documentation, to tell the
frontend dev to be careful. By adding a new loop in existing api we add a pit
to fall into. I really don't know what can be done about it though. Could we
get by with 1 second maybe?
> kauthaction.cpp:23
> #include <QtGlobal>
> #include <QRegExp>
>
I think that is deprecated, or at least not recommended for use anymore. You'll
want QRegularExpression
> kauthaction.cpp:53
> QVariantMap args;
> bool valid;
> QWidget *parent = nullptr;
Mh, minor annoyance. valid and timeout should get their defaults set here as a
more modern best practice and also because parent is already set for
consistency. That default ctor is kinda pointless then and should be dropped
IMHO.
REPOSITORY
R283 KAuth
REVISION DETAIL
https://phabricator.kde.org/D21795
To: feverfew, apol, bruns, davidedmundson, #frameworks, dfaure, cfeck, sitter,
chinmoyr
Cc: elvisangelaccio, bcooksley, ngraham, sitter, mreeves, kde-frameworks-devel,
LeGast00n, cblack, GB_2, michaelh, bruns
