GNOME Bugzilla – Bug 787246
libtracker-control: Let miner_manager_index_file accept a GCancellable
Last modified: 2017-09-11 11:45:09 UTC
Since tracker-2.0 is going to be a new major version with a new *.pc file name, it is a good to time to fix minor rough edges in the API. eg., tracker_miner_manager_index_file should accept a GCancellable.
Created attachment 359069 [details] [review] libtracker-control: Let miner_manager_index_file accept a GCancellable I haven't touched the libtool version string, though.
Comment on attachment 359069 [details] [review] libtracker-control: Let miner_manager_index_file accept a GCancellable I think makes sense to make the change under the 1.99.x/2.x umbrella. Luckily there's not that many users of the sync version of this call according to debian codesearch: https://codesearch.debian.net/search?q=tracker_miner_manager_index_file It would be nice to add some minimal description about it to libtracker-control API reference, the libtracker-miner similar doc could work as an inspiration: https://git.gnome.org/browse/tracker/tree/docs/reference/libtracker-miner/migrating-1to2.xml Would be nice to have that done together with this change, otherwise I'll add the docs blurb before next release.
Comment on attachment 359069 [details] [review] libtracker-control: Let miner_manager_index_file accept a GCancellable Pushed to master.
(In reply to Carlos Garnacho from comment #2) Thanks for the review! > Comment on attachment 359069 [details] [review] [review] > It would be nice to add some minimal description about it to > libtracker-control API reference, the libtracker-miner similar doc could > work as an inspiration: > https://git.gnome.org/browse/tracker/tree/docs/reference/libtracker-miner/ > migrating-1to2.xml I didn't realize that there is also a migration guide! That's nice. I will add something to it.
Created attachment 359171 [details] [review] libtracker-control: Add migration docs
Review of attachment 359069 [details] [review]: ::: src/libtracker-control/tracker-miner-manager.c @@ +1559,3 @@ g_return_val_if_fail (TRACKER_IS_MINER_MANAGER (manager), FALSE); g_return_val_if_fail (G_IS_FILE (file), FALSE); + g_return_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable)); Should be g_return_val_if_fail(). I’ve just pushed a fix as d6253a40. (I hope you don’t mind.)
Not at all :)
Comment on attachment 359171 [details] [review] libtracker-control: Add migration docs This one should go too before tomorrow's release. Thanks Debarshi :)
(In reply to Philip Withnall from comment #6) > Review of attachment 359069 [details] [review] [review]: > > ::: src/libtracker-control/tracker-miner-manager.c > @@ +1559,3 @@ > g_return_val_if_fail (TRACKER_IS_MINER_MANAGER (manager), FALSE); > g_return_val_if_fail (G_IS_FILE (file), FALSE); > + g_return_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable)); > > Should be g_return_val_if_fail(). > > I’ve just pushed a fix as d6253a40. (I hope you don’t mind.) Oops! That was embarrassing. Thanks for the fix.
Comment on attachment 359171 [details] [review] libtracker-control: Add migration docs Thanks for the review. Pushed to master.