[ADD] estate: Initial setup for module - #1369
Conversation
YassinWalid
left a comment
There was a problem hiding this comment.
Thanks for the impressive work. I have done an initial review, and left some comments in the PR and other general ones below. Please check them out, especially the coding guidelines and how to format the name/id of python fields and methods, xml record names and ids, as well as the model names themselves
Instead of date and datetime, you should use fields.Date and fields.Datetime
For the naming convention of records' name/id, name of models, fields, the order of methods for a model, and much more, check the coding guidelines for useful tips/rules.
5142161 to
15fcd01
Compare
|
Thank you for the many comments ! I really appreciate all the feedback, and tried to fix all the issues. I still have a warning from the ci about the translation, but should I care about it ? |
YassinWalid
left a comment
There was a problem hiding this comment.
Thanks for your work. I left some final comments on the PR, as well as some general ones below.
For the commit message titles, they should be [TAG] <module_name>: title. So in your case, they should have "estate" instead of "Real Estate". Also, the commit that introduces estate_account should have the tag [ADD] not [IMP].
Regardig the warnings, they're because translations should be done as self.env._ instead of _ since 18.0
`json` routes were deprecated to `jsonrpc` in 19.0, let's get rid of the warning to avoid confusion for the newdoos. task-none closes odoo#1037 Signed-off-by: Antoine Vandevenne (anv) <anv@odoo.com>
15fcd01 to
4e2016e
Compare
|
Thanks for the review, I implemented the necessary changes. |
YassinWalid
left a comment
There was a problem hiding this comment.
Thanks for your work.
One tiny further comment: For 528d7a9 the title should have "estate_account" instead of "estate", since this is the module you're adding.
6a27940 to
fc34842
Compare

Hello,
Happy to make my first PR in odoo.