Code review comment for lp:~mvo/unity-scope-click/lp1292645-use-libclick2

Thanks for working on this Michael.

You need to update to the tip of /devel. The configuration.{cpp,h} and associated tests have been moved under libclickscope/ now, so you'll need to update and resolve the conflicts by getting your changes under that directory instead.

The new libclick.{cpp,h} and associated tests should be placed in libclickscope/ instead, as well.

Also, I've noticed this pattern in your changes:

210 + try {
211 + ManifestList manifests = manifest_list_from_json(manifests_json);
212 + callback(manifests, ManifestError::NoError);
213 + } catch (...) {
214 + callback(ManifestList(), ManifestError::ParseError);
215 + }

This can be problematic, as any exceptions thrown inside callback() will result in callback() being called a second time with different arguments. I think it would be better to do this instead:

ManifestList manifests;
try {
    manifests = manifest_list_from_json(manifests_json);
} catch (...) {
    callback(ManifestList(), ManifestError::ParseError);
    return;
}
callback(manifests, ManifestError::NoError);

This way callback() will only be called once, and any exceptions it throws won't be lost inside the try block.