<table><tr><td style="">dvratil requested changes to this revision.<br />dvratil added a comment.<br />This revision now requires changes to proceed.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D19403">View Revision</a></tr></table><br /><div><div><p>Nice, thanks for the patch!</p>

<p>I haven't checked the code in-depth (I don't know if I'll get to it before the weekend, sorry), but one thing that I noticed: please update year and your name in the copyright headers :)</p>

<p>Second thing: I realized when I was fixing a bug in kgapi recently that having all the properties as strings everywhere (e.g. <tt style="background: #ebebeb; font-size: 13px;">QStringLiteral("fooPropertyName")</tt> is annoying) - it's hard to read and easy to make a typo in. Could you please define all the strings that are used as JSON property names and URL queries as named constants in an anonymous namespace at the top of the file, e.g. for TeamDrive:</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">namespace {
static const QString tdId = QStringLiteral("id");
static const QString tdName = QStringLiteral("name");
...
}</pre></div>

<p>Same for URL query parameters in jobs, e.g. <tt style="background: #ebebeb; font-size: 13px;">static const QString queryMaxResults = QStringLiteral("maxResults");</tt></p>

<p>I will eventually get around to change it everywhere to make the code safer. It will also reduce the size of the binaries since the strings won't be repeated twice there.</p>

<p>Thanks!</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R477 KGAPI Library</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D19403">https://phabricator.kde.org/D19403</a></div></div><br /><div><strong>To: </strong>barchiesi, dvratil<br /><strong>Cc: </strong>kde-pim, LibKGAPI, barchiesi, dvasin, rodsevich, winterz, vkrause, mlaurent, knauss, dvratil<br /></div>