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 762243 - Don't use gtk_status_icon_set_visible
Don't use gtk_status_icon_set_visible
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-02-18 08:44 UTC by Carlos Soriano
Modified: 2016-04-22 13:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Remove system tray icon handling (7.41 KB, patch)
2016-03-29 10:44 UTC, Ernestas Kulik
none Details | Review
Proposed patch v3 (7.53 KB, patch)
2016-03-30 08:52 UTC, Ernestas Kulik
none Details | Review
Proposed patch v4 (7.51 KB, patch)
2016-03-30 08:56 UTC, Ernestas Kulik
none Details | Review
Proposed patch v5 (7.57 KB, patch)
2016-03-30 10:06 UTC, Ernestas Kulik
accepted-commit_now Details | Review

Description Carlos Soriano 2016-02-18 08:44:48 UTC
Since it's deprecated. Find a replacement
Comment 1 Carlos Soriano 2016-02-18 08:45:38 UTC
And forgot to say, all the gtk_status_* functions!
Comment 2 Carlos Soriano 2016-03-09 17:12:33 UTC
update on this.
We actually assume where nautilus is running supports freedesktop notifications, so we can remove all the handling of systray icons, and therefore remove these funtions of gtk_status*.
Comment 3 Ernestas Kulik 2016-03-29 10:44:25 UTC
Created attachment 324935 [details] [review]
Remove system tray icon handling

This patch removes any mention of systray icons in the progress persistence handler. I left the checks for persistence in, FWIW.
Comment 4 Carlos Soriano 2016-03-30 08:23:52 UTC
Review of attachment 324935 [details] [review]:

Looks good so far, just missing some safe guards when the persistence is not present.

Also the commit message doesn't follow very well the commit guidelines. Look at https://wiki.gnome.org/Newcomers/CodeContributionWorkflow#Commit_guidelines

::: src/nautilus-progress-persistence-handler.c
@@ +57,2 @@
  * - when one file operation finishes, if it's not the last one, we only update the
+ *   resident notification's message

format the comment

@@ +151,2 @@
 static void
+progress_persistence_handler_hide_notification (NautilusProgressPersistenceHandler *self)

Missing safe guard
Comment 5 Ernestas Kulik 2016-03-30 08:52:37 UTC
Created attachment 324993 [details] [review]
Proposed patch v3
Comment 6 Ernestas Kulik 2016-03-30 08:56:22 UTC
Created attachment 324994 [details] [review]
Proposed patch v4

Forgot to include the safeguard.
Comment 7 Carlos Soriano 2016-03-30 09:48:42 UTC
Review of attachment 324994 [details] [review]:

Now code looks good, and commit message much better.

The commit message still lacks the file you modified "persistence-handler" in the title. See the commit messages guidelines.

Also, worth to mention in your last paragraph that we expect all DE that will use a nautilus as new as 3.22 to support persistence, and that's why we can safely remove it.
Comment 8 Ernestas Kulik 2016-03-30 10:06:21 UTC
Created attachment 324998 [details] [review]
Proposed patch v5

Ideally, this would be a one-shot deal. I now hope to not surpass five. :)
Comment 9 Ernestas Kulik 2016-04-18 07:06:00 UTC
Any update on this?
Comment 10 Carlos Soriano 2016-04-19 08:11:11 UTC
Review of attachment 324998 [details] [review]:

Whops, not sure why this bug emails goes to my spam folder and looked at it today by chance :/

Anyway, looks good now thanks!

Feel free to ping me on IRC when I forgot about reviewing something.