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 661231 - Minor cleanups for ShellUtil
Minor cleanups for ShellUtil
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2011-10-07 21:40 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2012-04-25 22:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
shell-util: Remove shell_util_icon_from_string (4.34 KB, patch)
2011-10-07 21:40 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
shell-util: Fix a bogus annotation (948 bytes, patch)
2011-10-07 21:40 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
telepathy-client: Replace shell_util_new_from_string (1.02 KB, patch)
2011-10-24 13:56 UTC, Florian Müllner
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2011-10-07 21:40:22 UTC
see patches
Comment 1 Jasper St. Pierre (not reading bugmail) 2011-10-07 21:40:24 UTC
Created attachment 198570 [details] [review]
shell-util: Remove shell_util_icon_from_string

GJS doesn't need to be able to represent interfaces for you to be able to
access an interface method, so Gio.icon_new_for_string works fine.
Comment 2 Jasper St. Pierre (not reading bugmail) 2011-10-07 21:40:27 UTC
Created attachment 198571 [details] [review]
shell-util: Fix a bogus annotation

Creating a new instance is not (transfer none), unless I'm missing
somehting here...
Comment 3 Giovanni Campagna 2011-10-08 15:08:23 UTC
Review of attachment 198570 [details] [review]:

Even though this reveals both a Gjs bug (should be able to represent Gio.Icon as a class object) and a gobject-introspection bug (new_for_string should be a constructor/static method for Gio.Icon), the patch is indeed correct.

(My accepted-commit_now is only an indication of the validity of the patch. I don't know where/when you can effectively commit)
Comment 4 Giovanni Campagna 2011-10-08 15:09:21 UTC
Review of attachment 198571 [details] [review]:

Seems correct...

Did you see a leak in valgrind?
Comment 5 Jasper St. Pierre (not reading bugmail) 2011-10-08 15:17:54 UTC
Review of attachment 198571 [details] [review]:

No, I was just staring at the code and saw this...
Comment 6 Jasper St. Pierre (not reading bugmail) 2011-10-11 04:08:34 UTC
Attachment 198570 [details] pushed as 786cfbd - shell-util: Remove shell_util_icon_from_string
Attachment 198571 [details] pushed as 67b7b7a - shell-util: Fix a bogus annotation


Pushed, thanks for reviewing!
Comment 7 Frederic Crozat 2011-10-24 13:43:18 UTC
there is still one Shell.util_icon_from left in telepathyClient.js, reopening (it is causing exception when loosing telepathy connections).
Comment 8 Florian Müllner 2011-10-24 13:56:22 UTC
Created attachment 199816 [details] [review]
telepathy-client: Replace shell_util_new_from_string

The function has been removed in commit 786cfbd3976, but one user
was overlooked when replacing it.

Here we go ...
Comment 9 Jasper St. Pierre (not reading bugmail) 2011-10-24 14:25:35 UTC
Review of attachment 199816 [details] [review]:

I couldn't find it because of my cleanup in bug #661236. Go for it.
Comment 10 Florian Müllner 2011-10-24 14:40:13 UTC
Attachment 199816 [details] pushed as b62f5ef - telepathy-client: Replace shell_util_new_from_string
Comment 11 Alban Browaeys 2012-01-26 11:58:14 UTC
gnome-3-2 branch is missing the fix namely b62f5ef07d87db7835fa753fe1774a5ed32d871d .
Comment 12 Frederic Crozat 2012-01-26 12:02:49 UTC
reopening, since it is still missing in 3.2 branch, could we get an approval for it ?
Comment 13 Florian Müllner 2012-01-26 12:06:56 UTC
(In reply to comment #12)
> reopening, since it is still missing in 3.2 branch, could we get an approval
> for it ?

I guess, but there are no plans for further 3.2.x releases ...