[Sugar-devel] Proxy Settings in Network Control Panel

[Manuel Quiñones]

Oh and forgot to say: in my commit message I tried to respect all the
information from the original patch, and added new paragraphs to
describe the new implementation.  If you see anything I missed, please
tell me.

2013/3/13 Manuel Quiñones <manuq at laptop.org>:
> Hi Ajay and other devs,
>
> Ajay, whlie reviewing your patch for the proxy feature, I decided to
> step in and work on the issues I've found by myself.  I did this to
> speed up the process, instead of going through a long
> review/patch/review loop.  I hope you understand.  A (rather long,
> sorry) description of what I did follows.  But first of all, I would
> be glad if you can test my last patch:
>
> https://bugs.sugarlabs.org/attachment/ticket/3070/0001-Add-proxy-configuration-support-to-Network-Control-P.patch
>
> The first thing I've found is that your patch does both GConf and
> GSettings.  I decided to go just for GSettings because Browse and all
> underlayng GTK use it.  In any case I think you should have added
> another mixin class for GSettings instead of adding code to GConfMixin
> class.  Sasha's original patch had that quite separated to make adding
> another backend an easy task.
>
> So my first attempt was doing only GSettings and fixing the issues
> I've found in yours:
>
> https://bugs.sugarlabs.org/attachment/ticket/3070/proxy_gsettings.patch
> (note this is not my final patch, let's call this my a) patch)
>
> An issue with your patch: the UI does not always update when a setting
> is changed.  TestCase: change a setting, quickly close the network
> section and open it again: you will see the original value.  This is
> because the timeout that calls _commit has not finished yet.  Sasha's
> patch had gconf client.notify_add() for this.  In my a) patch I have
> added a callback to the 'changed' event of gsettings that does the
> same.
>
> https://developer.gnome.org/gio/stable/GSettings.html#GSettings-changed
>
> There are other differences between Sasha's patch and yours.  For
> example you seem to forgot the try/finally clause in the _commit
> method that blocks the __changed_cb callback.  I have added it back in
> my a) patch.
>
> I intentionally skipped $http_proxy environment variable (Sasha's patch 3/3):
>
> http://lists.sugarlabs.org/archive/sugar-devel/2012-August/038804.html
>
> This could be added later if there is a reason.  I don't see GNOME
> doing it.  Which activities or parts of Sugar need it?  Indeed this
> makes wget work with proxy by default but then why GNOME does not do
> it?  Am I missing any conversation?
>
> So, now for my final patch.  After having my a) patch working, I took
> another path using the convenient way GSettings provides to bind
> widgets and settings:
>
> https://developer.gnome.org/gio/stable/GSettings.html#g-settings-bind
>
> The adventages can be seen in the ~100 lines removed.  There is...
>
> - no need for a mixin class,
>
> - no need for connecting callbacks to signals between widgets and
> settings in both directions
>
> - no need to implement get_value_for_gsettings /
> set_value_from_gsettings for each widget.  Sasha's gconf
> implementation needed a big hierarchy of classes to archive this.
>
> - no need to test the key type to map the corresponding getter ('s' to
> use get_string(), for example, compare with my a) patch)
>
> So, I welcome testing in different proxy scenarios for my patch.
> Thanks a lot Ajay for your contributed work, which I took as basement.
>
> --
> .. manuq ..



-- 
.. manuq ..