[Differential] [Requested Changes To] D3395: Patch for GitHub is never authorized Kubuntu 16.10 no crash (bug 372144)

[kfunk (Kevin Funk)]

kfunk requested changes to this revision.
kfunk added a reviewer: kfunk.
kfunk added a comment.
This revision now requires changes to proceed.


  Thanks for working on this David! Much appreciated.

INLINE COMMENTS

> ghdialog.cpp:118
>  
> -void Dialog::authorizeResponse(const QByteArray &id, const QByteArray &token)
> +void Dialog::authorizeResponse(const QByteArray &id, const QByteArray &token, QString &tokenName)
>  {

`const QString&` please

> ghdialog.cpp:128
>          m_account->setName(QString());
>          KMessageBox::sorry(this, i18n("Authentication failed! Please, "
> +                                      "try again " "couldn't create token: ")+tokenName+"\n"+

Would be better to give this dialog a proper caption. There's a multi-QString-arg version of KMessageBox, see here: https://api.kde.org/frameworks/kwidgetsaddons/html/namespaceKMessageBox.html#a02dfebea27d2b3e28199aa543a4d012c.

Set both caption + text. Make sure casing is correct as well.

> ghresource.cpp:65
>      url.setPath(url.path() + '/' + "/authorizations");
> -    QByteArray data = "{ \"scopes\": [\"repo\"], \"note\": \"KDevelop Github Provider\" }";
> +    m_tokenName = "KDevelop Github Provider : "+QHostInfo::localHostName()+" - "+QDateTime::currentDateTimeUtc().toString();
> +    QByteArray data = "{ \"scopes\": [\"repo\"], \"note\": \""+m_tokenName.toUtf8()+"\" }";

Style: `"" + a + b + ...` (spaces between operator & operands)

> ghresource.cpp:151
>          emit authenticated(map.value("id").toByteArray(),
> -                           map.value("token").toByteArray());
> +                           map.value("token").toByteArray(),m_tokenName);
>      } else

Style: Space after comma

> ghresource.cpp:153
>      } else
> -        emit authenticated("", "");
> +        emit authenticated("", "",m_tokenName);
>  }

Dito, same in other places

REPOSITORY
  rKDEVELOP KDevelop

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: WDavidO, #kdevelop, kfunk
Cc: kfunk, kdevelop-devel, Pilzschaf, akshaydeo, surgenight, arrowdodger
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20161117/f8e1e973/attachment-0001.html>