D19948: DRAFT Add Standard Query Parameters support

[Daniel Vrátil]

dvratil requested changes to this revision.
dvratil added a comment.
This revision now requires changes to proceed.


  Sorry, I forgot to do a second round of review.

INLINE COMMENTS

> job.cpp:436
> +
> +void Job::setPrettyPrint(bool &prettyPrint)
> +{

`prettyPrint` doesn't need to be (shouldn't be!) a reference.

> job.cpp:472
> +    }
> +    query.addQueryItem(PrettyPrintParam, d->prettyPrint ? QStringLiteral("true") : QStringLiteral("false"));
> +

Do you maybe need to adjust /all/ tests, now to reflect this?

> about.h:224
>  
> -    explicit About(const About &other);
> -    virtual ~About();
> +    static const QString AdditionalRoleInfoField;
> +    static const QString AdditionalRolesField;

I'm not sure how I feel about this barrage of members, if nothing else this will pollute auto-completer in the IDEs :-)

What do you think about this?

  class About {
      ....
      struct Fields {
          static const QString AdditionalRoleInfo;
         ...
      };
  };

Then the usage would be `About::Fields::AdditionalRoleInfo`, which feels cleaner and won't pollute `About` itself that much.

REPOSITORY
  R477 KGAPI Library

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

To: barchiesi, dvratil
Cc: kde-pim, #libkgapi, barchiesi, gennad, dvasin, rodsevich, winterz, vkrause, mlaurent, knauss, dvratil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20190328/d8b5dcb6/attachment.html>