Hello!
Piotr Sipika has written on Saturday, 11 July, at 22:46:
>On 08/01/2014 08:33 PM, Andrej N. Gritsenko wrote:
>> 4) might it be better to use conditional compilation instead of using
>> conditional execution? That way we can omit some unused code when it's
>> used as panel plugin.
>It looks like that's what you did on master. I completely removed the
>app-specific code from the plugin -- it was just taking up space. The
>proper way of reusing code would have been for me to generate a shared
>library and go from there. Thanks for cleaning it up.
Well, code with #ifdef does take any space after preprocessor really,
but you still could use that code for your standalone application without
copying it around. But that's your decision of course.
>I've also fixed:
> - the threading issue brought up by Henry Gebhardt 2 1/2 (!!!) years
>ago (essentially with a slow network, the weather plugin would slow down
>lxpanel's startup) [2]
> - the forecast for 'Tomorrow' was actually the forecast for 4 days from
>'Today', due to a parsing bug [3] (the fix also effectively adds a
>requested feature [4]).
>All of the above fixes to the plugin are available for a 'pull' from the
>'1-threading' branch of my fork of the lxpanel repo [5]
Thank you very much for all the work done. Although there is a small
note on it. The commit where many different changes are all-in-one is
very hard to be understood, to review ang get commented, and your biggest
commit has at least 5 things in it: a) indentations changes; b) many data
members renamed; c) bugs fixed; d) forecast feature improvement; e) fix
for hang due to data retrieval in main thread. So in short, I have no big
idea what to comment on it so I think it should be just merged with the
master after we done with 0.8.x and will prepare 0.9.0. I hope you would
take this into consideration on your further work and would split your
further commits into few more atomic ones (i.e. with one kind of change
in each) so they can be reviewed and commented.
With best regards,
Andriy.
------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
Lxde-list mailing list
Lxde-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxde-list
