maorb - Technical Training#1286
Conversation
785790e to
55faeb6
Compare
Megaaaaaa
left a comment
There was a problem hiding this comment.
Hello 👋
Very nice PR, good job on this! 👍
There's quite a lot of stuff but don't worry about it, it's a lot of the same small things that repeat themselves.
There is a lot of stuff on your ci/style so usually you would start with that but I would recommend you have a look at my review first because I might explain why the ci/style flags things that you would otherwise think are fine.
Keep in mind that a review is not the utlimate solution, it's only someone seeing things he would have done differently and offering an alternative. You're always free to answer with an other alternative or even say you disagree and bring your arguments. For stuff that's not just styling or conventions, don't tak the wrong habit of just blindly applying comments, you spent time developping the thing, everything you've tried doesn't show up in the diff, only the final one. You probably have very interesting things to say to the reviewer about the decisions you made.
What I would suggest you do and continue to do while you're not the most comfortable with odoo's structure is to not apply all the changes at once. For example, changing a compute method or even just indentation can break stuff and you change everything at once, you might end up with a lot of errors when running the database. Splitting the review in multiple steps and trying to run your db in between those can save you a lot of time in the long run. Also don't be shy with dropping the databse. It's a very nice way to make sure everything works well from scratch.
Also a quick tip for the methodology when applying reviews changes: What I like to do it put a reaction like a thumbsup on a message once I have made the change locally. Then when everything is marked with something, I push, go through my diff and mark as resolved the things that are now outdated with the button under each comment so nothing is left forgotten.
If you have any question, don't hesitate to ask!
4447604 to
36e6b39
Compare
b8fd555 to
a46b23f
Compare
…Invoicing Problem: There is currently no module to manage selling properties. Solution: Two new modules create a Real Estate application which manages multiple properties, handles offers, and creates invoices once properties are sold. Task: 6229439
e7ab1c0 to
99e5de2
Compare

No description provided.