https://bugs.freedesktop.org/show_bug.cgi?id=83396

--- Comment #4 from Patrick Ohly <[email protected]> ---
I've split up the patch a bit and fixed a few things (extra white space at end
of lines, missing API change in getOAuth2Bearer from signon.cpp, formatting and
text of the README, compiler warnings about TransportAgent::CLOSED and
INACTIVE, conditional compilation, compile flags).

I renamed GVariant.[h/cpp] to GVariantSupport (to be consistent with other C++
utility files for C APIs) and dropped the hand-written auto_ptr in favor of the
more flexible shared CXX wrapper. json.pc seems to be a legacy library
(according to Debian Testing); I made the code compile with it and json-c.pc,
the successor.

This is all just nitpicking. The core idea and implementation is okay. Thanks
for the patch. I'll attach my revised patch series for review.

One thing I am uncertain about is the use of "refresh token" in the identity
provider name. It's surprising that the "oauth2" backend installs a
providerrefreshtoken.so. I think I would prefer to have refresh[_-]?token
replaced by simply oauth2. Agreed?

The handling of the command-line-only case without a permanent config was
missing. It's a bit hard to test because it only triggers when we need a new
refresh token, and I haven't seen that happen. I suspect what would happens is
an exception from a non-writable config node, which is covered by the general
catch all.

I have also not tested how a failure to get an OAuth2 bearer token is reported.
In particular I cannot guarantee that a sync fails with an error that indicates
an authentication problem.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
Syncevolution-issues mailing list
[email protected]
https://lists.syncevolution.org/mailman/listinfo/syncevolution-issues

Reply via email to