GNOME Bugzilla – Bug 682721
Add support for gresource URIs
Last modified: 2012-10-20 09:36:17 UTC
libsoup could automatically handle requests to resource:// URIs
Created attachment 222460 [details] [review] Patch Add support for greesource URIs to SoupRequestFile. Patch includes tests for file, data and resource requests.
Review of attachment 222460 [details] [review]: Overall LGTM. Let's see what Dan thinks about it. ::: libsoup/soup-request-file.c @@ +134,3 @@ + uri_str = g_strdup_printf ("resource://%s", decoded_path); + file->priv->gfile = g_file_new_for_uri (uri_str); + g_free (uri_str); Maybe it'd make sense to define "resource://" as intern string in soup-uri as we do with "http://" or "data://" ::: tests/requests-test.c @@ +25,3 @@ + +static void +regsiter_gresource (void) register_gresource () I guess @@ +159,3 @@ + + get_index (); + regsiter_gresource (); register_gresource(); @@ +161,3 @@ + regsiter_gresource (); + + session = soup_test_session_new (SOUP_TYPE_SESSION_SYNC, NULL); We normally add tests for both synchronous and asynchronous sessions
Created attachment 226798 [details] [review] Updated patch Updated pach. Fixes the typo in tests, add async version of the tests and adds "resource" as an intern string.
Comment on attachment 226798 [details] [review] Updated patch basically good. 3 notes: 1) make sure that "make distcheck" still passes after all your resource-related changes to tests/Makefile.am. 2) rename "requests-test" to "resource-test", since that describes it better, and so I don't get it confused with "requester-test" :) 3) fix up the tabs-vs-spaces confusion in the new test .c file. (It should use tabs.) OK to commit after that; let's get this in before 3.6.1 on Monday. (And sorry for not reviewing it sooner.)
(In reply to comment #4) > (From update of attachment 226798 [details] [review]) > basically good. 3 notes: > > 1) make sure that "make distcheck" still passes after all your resource-related > changes to tests/Makefile.am. Sure. > 2) rename "requests-test" to "resource-test", since that describes it better, > and so I don't get it confused with "requester-test" :) Actually I used requests, because it also tests file and data requests in addition to resources, but I agree it easy to get it confused with requester-test. > 3) fix up the tabs-vs-spaces confusion in the new test .c file. (It should use > tabs.) M-x tabify will do the trick > > OK to commit after that; let's get this in before 3.6.1 on Monday. (And sorry > for not reviewing it sooner.) Great, I guess you mean 3.7.1 :-)
Comment on attachment 226798 [details] [review] Updated patch Renamed to resource-test, M-x tabify, fixed distcheck and pushed to git master. Thanks!