Tryton - Issues

 

Issue4735

Title Allow to Apply inheritance on models loaded in the pool
Priority feature Status testing
Superseder Nosy List ced, pokoli, reviewbot
Type feature request Components trytond
Assigned To pokoli Keywords review
Reviews 15231002
View: 15231002

Created on 2015-05-01.13:53:51 by pokoli, last changed by reviewbot.

Messages
review15231002 updated at https://codereview.tryton.org/15231002/#ps120001
review15231002 updated at https://codereview.tryton.org/15231002/#ps100001
review15231002 updated at https://codereview.tryton.org/15231002/#ps80001
msg31520 (view) Author: [hidden] (pokoli) (Tryton committer) Date: 2017-01-27.13:43:45
I updated the review taking in account msg31265
msg31265 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-01-13.16:00:22
I think the design should be simpler.
Here it is:

- register in the pool Mixin with a list of classes
- at the end of the load_module_graph:
  - the pool is called to loop over all the classes
  - test if it is a subclass of all the classes registered with the Mixin.
  - add the Mixin to the pool class

I think it is a much simpler design and there is no need to have the Mixin applied before having loaded all the modules. And if there are cases where it is needed, then the module can just explicitly extend the needed classes with the Mixin.
review15231002 updated at https://codereview.tryton.org/15231002/#ps40001
msg21253 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2015-05-18.12:36:54
That's a very bad idea: explicit is better than implicit.
You can have name collision between different class type and even worst different signature for the same named method.
msg21252 (view) Author: [hidden] (pokoli) (Tryton committer) Date: 2015-05-18.12:20:45
I dont think it's necessary to specify what class you want to override, as in the code you can test issubclass or isinstance, so you know if the class is a part of a mixin.
msg21180 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2015-05-06.23:20:44
It will be good to also be able to extend any Mixin.
msg21147 (view) Author: [hidden] (pokoli) (Tryton committer) Date: 2015-05-01.13:53:50
See https://groups.google.com/forum/#!topic/tryton-dev/CeoBg6iAH7I
History
Date User Action Args
2017-02-17 15:10:03reviewbotsetmessages: + msg32037
2017-02-13 18:03:53reviewbotsetmessages: + msg31891
2017-02-10 13:30:54pokoliunlinkissue5907 superseder
2017-01-30 15:30:20pokolisetstatus: in-progress -> testing
2017-01-27 14:17:19reviewbotsetmessages: + msg31521
2017-01-27 13:43:45pokolisetmessages: + msg31520
2017-01-13 16:00:22cedsetmessages: + msg31265
2016-12-20 12:03:20reviewbotsetnosy: + reviewbot
messages: + msg30836
2016-10-26 18:49:22cedlinkissue5907 superseder
2015-05-18 12:36:55cedsetmessages: + msg21253

Showing 10 items. Show all history (warning: this could be VERY long)