Review of attachment 274456 [details] [review]:

That patch is certainly more controversial, since you are adding public header and an API dependency on libsoup.

I don't know the opinion of the maintainer, but it could be more appropriate to add a new optional header, such as "rest-soup.h", that would limit the Soup API exposure. 

Please also add comment and fill the gtk-doc section file.

Created attachment 284427 [details] [review]
Add rest_proxy_add_soup_feature()

This function can be helpful if one wants more control over libsoup
features than what librest simple API provide. For example, to get full
access to libsoup cookie API (say to be able to add arbitrary cookies to
the soup session), one can do:

RestProxy *proxy = g_object_new(REST_TYPE_PROXY,
                                "url-format", url,
                                "disable-cookies", TRUE,
                                NULL);
SoupSessionFeature *cookie_jar = SOUP_SESSION_FEATURE(soup_cookie_jar_new ());
rest_proxy_add_soup_feature(proxy, cookie_jar);

It's then possible to use all the soup_cookie_* methods to deal with
cookies.

Review of attachment 284427 [details] [review]:

I haven't put the new function in a separate header, I'm waiting on feedback from the maintainers as to how they want to expose this new method before doing so.

Review of attachment 284427 [details] [review]:

ebassi is fine with soup include in rest.h

11:11 < elmarco> ebassi: about #728340, any opinion about having soup include on public headers, or should a 
                 new rest-soup.h file be added for Soup-specific api?
11:21 <@ebassi> elmarco: if the API exposes Soup types, then there's nothing wrong in including it in an 
                installer header — actually, that's what I'd expect
11:31 < elmarco> ebassi: the question was not whether include of Soup is required, but rather if it should be 
                 in the default "rest.h" header or a specific rest-soup.h, to limit Soup API exposure, since 
                 rest didn't explicitely depend on Soup so far
11:32 < elmarco> (well, it is in .pc Requires: although, there is no reason iirc, it should rather be in 
                 Requires.private)

11:38 <@ebassi> elmarco: I don't think we gain anything by hiding the fact that you'll likely need Soup to use 
                librest
11:38 <@ebassi> it's not like people are asking for a curl code path
11:39 <@ebassi> and if librest needed to change implementation, it would still require an API bump
11:39 <@ebassi> so, no: I don't think it's worth it adding a new rest-soup.h header
11:39 <@ebassi> just for one symbol
11:40 <@ebassi> to be fair, some of the API decisions made in librest are pretty horrible, and come from a bad 
                place
11:40 <@ebassi> I'd probably rework the whole API, if I had time

Attachment 284427 [details] pushed as 68d90e9 - Add rest_proxy_add_soup_feature()

Created attachment 285156 [details] [review]
Add libsoup to gir includes

Since the public API now references a type from libsoup, we need
to add Soup-2.4.gir to Rest_@API_VERSION_AM@_gir_INCLUDES.

Review of attachment 285156 [details] [review]:

ack

Comment on attachment 285156 [details] [review]
Add libsoup to gir includes

Attachment 285156 [details] pushed as 0af8d4d - Add libsoup to gir includes