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 648648 - [PATCH] simplify connection filtering logic
[PATCH] simplify connection filtering logic
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: network-indicator
3.0.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2011-04-25 22:11 UTC by Dan Williams
Modified: 2011-05-04 05:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Use libnm-glib functions instead of hand-rolling (5.58 KB, patch)
2011-04-25 22:13 UTC, Dan Williams
reviewed Details | Review
Updated patch simplifying the logic (8.76 KB, patch)
2011-04-27 17:53 UTC, Dan Williams
accepted-commit_now Details | Review

Description Dan Williams 2011-04-25 22:11:00 UTC
I added some code to libnm-glib as of 779215c742bbe29a2c66202ec7e2e6d43edeb8ff so we can kill that code from network.js and just use the libnm-glib utility functions.  The code in libnm-glib was already present as the *_filter_connections() functions, but network.js usually only checks one connection instead of a list of them.  So instead of making network.js build up a single-item list just make libnm-glib better.
Comment 1 Dan Williams 2011-04-25 22:13:16 UTC
Created attachment 186623 [details] [review]
Use libnm-glib functions instead of hand-rolling
Comment 2 Giovanni Campagna 2011-04-26 12:18:03 UTC
Review of attachment 186623 [details] [review]:

It is surely correct as is, but I think you can remove connectionValid from all subclasses, and leave it in NMDevice base class.

By the way, this should also fix downstream bug https://bugzilla.redhat.com/show_bug.cgi?id=693151 (as now NMDeviceEthernet incorrectly ignores pppoe connections) so if possible it should be part of 3.0.2.
Comment 3 Dan Williams 2011-04-27 17:53:29 UTC
Created attachment 186768 [details] [review]
Updated patch simplifying the logic

Updated patch removes all the unecessary subclass connectionValid() calls, and converts the connectionValidForAP() calls directly into ap.connection_valid() instead.
Comment 4 Dan Williams 2011-04-28 21:23:34 UTC
FYI a version of NM with the necessary libnm-glib changes hit f15 and rawhide yesterday:

https://admin.fedoraproject.org/updates/NetworkManager-0.8.998-4.git20110427.fc15
Comment 5 Colin Walters 2011-04-28 22:03:09 UTC
Review of attachment 186768 [details] [review]:

If you've tested this, looks good to me.
Comment 6 Dan Williams 2011-05-04 05:35:29 UTC
c31109800b3267df433841bff08c9383a5d669cb

and configure.ac updated to require NM 0.8.999 which has the necessary functions and was released today as well.