D29017: WIP: Introduce three new methods that return all "entries" in a folder

David Faure noreply at phabricator.kde.org
Wed Apr 22 14:43:22 BST 2020


dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kwallet.cpp:219
> +                KSecretsService::ReadItemPropertyJob *readLabelJob = item->label();
> +                if (readLabelJob->exec()) {
> +                    const QString label = readLabelJob->propertyValue().toString();

Urgh. This actually calls for async API instead.

But now I'm really confused. HAVE_KSECRETSSERVICE is never set anywhere (except to 0), how does one even compile this code?

> kwallet.h:382
> +     *         is the entry key.
> +     *  @return Returns 0 on success, non-zero on error.
> +     *

Are there documented error codes somewhere? Otherwise a bool would do be more readable, no?

But then if we don't have error codes, wouldn't

  QMap<QString, QByteArray> entriesList()

be better?

> kwalletbackend.cc:558
> +    const EntryMap &map = _entries[_folder];
> +    rc.append(map.values());
> +

`rc = map.values()`, append() makes little sense if it's empty.

> kwalletbackend.h:140
> +    // Get a list of all the entries in the current folder.
> +    // You delete the list.
> +    // @since 5.70

This makes no sense. The list is a value. Does this mean "The caller takes ownership of the entries"?

REPOSITORY
  R311 KWallet

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

To: ahmadsamir, #frameworks, dfaure
Cc: blaze, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200422/1d91c5f4/attachment.html>


More information about the Kde-frameworks-devel mailing list