GNOME Bugzilla – Bug 701500
Photo support for Facebook provider
Last modified: 2013-09-10 12:46:23 UTC
Hi! I'm working in the Facebook Photo support for GNOME Photos and I'm using the GOA as the authorizer, so I need to add some new authorizations and other things.
Created attachment 245899 [details] [review] The few lines to allow photo support to the Facebook provider
Review of attachment 245899 [details] [review]: Thanks for the patch. Looks good, apart from a few minor things. ::: src/goa/Makefile.am @@ +119,3 @@ typelibs_DATA = Goa-1.0.typelib Goa-1.0.gir: $(libgoa_1_0_la_SOURCES) It would be nice to put this into a separate patch. ::: src/goabackend/goafacebookprovider.c @@ +143,2 @@ } We also have to bump the value returned by the get_credentials_generation vfunc. This is needed because older access_tokens won't work with the new set of scopes.
Created attachment 245909 [details] [review] facebook: Add support for photos
Created attachment 245910 [details] [review] build: Don't hard code the paths to girdir and typelibdir
Thank you very much for your work on improving the integration with Facebook.
(In reply to comment #4) > Created an attachment (id=245910) [details] [review] > build: Don't hard code the paths to girdir and typelibdir Hm, that patch and especially the commit message doesn't make sense. What the patch does is *actually* hard-code the gir and typelibdir paths instead of getting the proper paths from gobject-introspection-1.0.pc. The two variables are defined as INTROSPECTION_GIRDIR=`$PKG_CONFIG --variable=girdir gobject-introspection-1.0` INTROSPECTION_TYPELIBDIR=`$($PKG_CONFIG --variable=typelibdir gobject-introspection-1.0)`
(In reply to comment #6) > (In reply to comment #4) > > Created an attachment (id=245910) [details] [review] [details] [review] > > build: Don't hard code the paths to girdir and typelibdir > > Hm, that patch and especially the commit message doesn't make sense. The commit message also just mentions what was changed, but not why. Maybe it would help to elaborate on what problem this patch was supposed to fix.