[Sugar-devel] Proxy Settings in Network Control Panel
manuq at laptop.org
Wed Mar 13 22:03:25 EDT 2013
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
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:
> 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:
> (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
> 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):
> 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:
> 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 ..
More information about the Sugar-devel