After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 772255 - gresolver: Mark GResolver as an abstract class
gresolver: Mark GResolver as an abstract class
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: network
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2016-09-30 13:21 UTC by Philip Withnall
Modified: 2016-10-04 15:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gresolver: Mark GResolver as an abstract class (1.37 KB, patch)
2016-09-30 13:21 UTC, Philip Withnall
committed Details | Review

Description Philip Withnall 2016-09-30 13:21:03 UTC
Trivial patch to mark GResolver as abstract. This breaks API, but arguably nobody could have been using the API in the way this breaks, so there should be no impact. I would be happy for the patch to be rejected for breaking API though.

The motivation is to prevent people accidentally shooting themselves in the foot by using (e.g., in Python);
   resolver = Gio.Resolver()
rather than
   resolver = Gio.Resolver.get_default()

It’s a lot harder to get this wrong in C, since g_resolver_new() doesn’t exist. You’d have to do:
   resolver = g_object_new (G_TYPE_RESOLVER, NULL);
rather than
   resolver = g_resolver_get_default ();
Comment 1 Philip Withnall 2016-09-30 13:21:07 UTC
Created attachment 336670 [details] [review]
gresolver: Mark GResolver as an abstract class

It only works through its virtual methods, so while it could be
instantiated before (and this is technically an API break), any instance
of it would previously have crashed as soon as any of its methods were
called anyway.
Comment 2 Emmanuele Bassi (:ebassi) 2016-09-30 14:24:10 UTC
I think this is an acceptable ABI change, given the way GResolver is meant to be used.
Comment 3 Philip Withnall 2016-10-03 23:32:24 UTC
Dan, what do you think (since you wrote GResolver)?
Comment 4 Dan Winship 2016-10-04 12:17:27 UTC
Sure
Comment 5 Philip Withnall 2016-10-04 15:04:23 UTC
Thanks for the fast replies. Merged.

Attachment 336670 [details] pushed as 10bac98 - gresolver: Mark GResolver as an abstract class