Code review comment for lp:~alecu/unity-lens-music/ubuntuone-webservices

56 + public unowned string sign_url2(string url, string? postargs,

Looking at the docs, it says that you need to free the return, so s/unowned//, plus the postargs should be "out string postargs" as it's also a return (out params are nullable by default).

118 + internal Soup.SessionAsync http_session;

"internal" is a special keyword used when building libraries in vala (internal symbols are callable by other parts of the library, but not by the consumers), pls use private/public as appropriate.

177 + public string nickname {
178 + get { return _nickname; }
179 + }
+ other instances

No need for the extra private variable, use "public string x { get; private set; }"

243 + credentials_management.find_credentials ();
244 + yield;

Wouldn't it be better if find_credentials() itself was async? Then you wouldn't need the signals and you could just return the credentials / throw an error in the async method.

275 + internal virtual async void fetch_account () throws PurchaseError

Nothing wrong here, nice async method ;)

285 + var result = (string) message.response_body.data;

278 + queue_signed_webservice_call ("GET", ACCOUNT_URI, (session, message) => {

You're using this multiple times, perhaps add a helper async method that takes method + uri and returns flattened Soup.MessageBody?

.data could be null, no? Or does the flatten() cause possible null to become ""?

576 + return 0;

You should return result of Test.run ()

852 + MainLoop mainloop = new MainLoop ();
853 + purchase_service.refetch_payment_info.begin((obj, res) => {
854 + mainloop.quit ();
855 + try {
856 + purchase_service.refetch_payment_info.end (res);
857 + } catch (PurchaseError e) {
858 + error ("Can't fetch payment info: %s", e.message);
859 + }
860 + });
861 + mainloop.run ();

I remember you mentioned on IRC that you weren't sure how to do async tests with vala, so yep, this is the way, usually we just have a wrapper for the mainloop.run() which also adds a timeout, so the test fails eventually if there's no response. Usually using "assert (run_with_timeout (mainloop));"