D2621: GitHub Two Factor Authentication

[Kevin Funk]

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


  Rest LGTM

INLINE COMMENTS

> ghresource.cpp:173
> +        const auto& header = metaData[QStringLiteral("HTTP-Headers")];
> +        if (header.contains(QStringLiteral("x-github-otp: required;"), Qt::CaseSensitive)) {
> +          m_tfHttpHeader = qobject_cast<KIO::StoredTransferJob*>(job)->outgoingMetaData()[QStringLiteral("customHTTPHeader")];

`"x-github-otp: required`: Still incorrect casing. Does this work?

https://developer.github.com/v3/auth/ says it's `X-GitHub-OTP: required;`

> ghresource.cpp:174
> +        if (header.contains(QStringLiteral("x-github-otp: required;"), Qt::CaseSensitive)) {
> +          m_tfHttpHeader = qobject_cast<KIO::StoredTransferJob*>(job)->outgoingMetaData()[QStringLiteral("customHTTPHeader")];
> +          emit twoFactorRequested();

Please pass `m_tfHttpHeader` via `twoFactorRequested()` instead I.e. add a parameter to that function. That way you can save the extra member in this class for `m_tfHttpHeader`.

And in general: Please refrain from adding less known abbreviations to symbol names; it makes reading code harder.

> ghresource.h:145
> +     */
> +    void twoFactorRequested();
> +

Rename: `twoFactorRequested` -> `twoFactorAuthRequested`

REPOSITORY
  R32 KDevelop

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

To: zhigalin, mwolff, kfunk, ial0
Cc: mwolff, kfunk, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170802/ef3733da/attachment-0001.html>