Review: Needs Fixing Hello Thierry,
Thanks for your valuable contribution ! Just a few comments ! There's always a big risk associated when you make a big change, risk like loosing essential features, regressions etc. Its should be tested and well executed keeping in mind the previous functionalities offered. By loosing essential features I meant that Currently OpenERP allows you to define your own extensions from the extension manager options from the toolbar(GTK client) options/extension manager for eg: I would like to open/print *XYZ* extension files with firefox. Then OpenERP gives higher priority to the extensions defined by the user if found use them else go for the other openers according to extension type. So with your merge proposal this features gets lost. secondly: OpenERP allows users to define their softpath(usually for pdf) and softpath_html(usuallyfor html types) i.e If you want to use another PDF viewer or if you do not want to use the first one the OpenERP will find for you, you have to edit the OpenERP configuration file, normally located in ~/.openerprc and in the [printer] section edit the softpath and softpath_html parameter. This feature is also lost with your merge proposal. Thirdly: can you please refactor your code please like remove the unused imports, unnecessary dict(self.openers) can be just a list as they all gives the same self._findOpener function. please before making such changes please have a deep look at the existing system working and the features it provides. For eg: if you remove a line then first just think why was the line introduced. do I cover the intension in my new proposal etc. Once again thanks for your work and patience ! Hope to see the improved merge soon. Regards, -- https://code.launchpad.net/~openerp-community/openobject-client/zehk_use-of-xdg-open/+merge/77333 Your team OpenERP Community is subscribed to branch lp:~openerp-community/openobject-client/zehk_use-of-xdg-open. _______________________________________________ Mailing list: https://launchpad.net/~openerp-community Post to : [email protected] Unsubscribe : https://launchpad.net/~openerp-community More help : https://help.launchpad.net/ListHelp
