-
Notifications
You must be signed in to change notification settings - Fork 2.9k
training-tupan #1124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 19.0
Are you sure you want to change the base?
training-tupan #1124
Conversation
2ee5a6f to
52a1b26
Compare
plha-odoo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job for the general quality and readability of the code !
A few comments :
- We try to always leave an empty line at the end of every file.
If you are using vscode there is an option to automatically do it which is called Insert Final Newline. - for the commit message and title we try to always follow these guidelines https://www.odoo.com/documentation/16.0/contributing/development/git_guidelines.html
- a few styling modifications for which I made local reviews (spaces arround an '=' except when used to define a parameter, spaces after but not before a ':', etc)
plha-odoo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job for these chapters, for the applying the correct styling almost everywhere and for udpating the code after the first review.
Just a few comments :
- don't forget to use the proper git commit title convention event if it's an udpate, in which case your title should start with [FIX]
- two styling for which I did local reviews
Co-authored-by: plha-odoo <plha@odoo.com>
Co-authored-by: plha-odoo <plha@odoo.com>
Co-authored-by: plha-odoo <plha@odoo.com>
Complete chapter 8 Fix naming for view fiels (estate_views.xml -> estate_view.xml) Remove unsupported '_sql_constraints' from estate_property model Add missing author and license keys in manifest to remove the warnings
eff76ae to
98144b2
Compare
|
Performed a rebase to align commit history with git guidelines. |
plha-odoo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job on applying the previous reviews and on adapting the previous git commit titles !
I left a few reviews in the code but mostly nitpicking
plha-odoo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job on applying the previous reviews and on adapting the previous git commit titles !
I left a few reviews in the code but mostly nitpicking
…ions, stat button for offers inside estate property type (CHAPTER 11)
… is sold (CHAPTER 13)
4ddb029 to
ae3a717
Compare
[FIX] estate: fix bug - error for selling price lower than 90% of expected price when creating a new property (selling_price is zero)
plha-odoo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job ! just a single styling improvement but it's nitpicking
estate/views/estate_form_view.xml
Outdated
| </field> | ||
| </record> | ||
| </odoo> | ||
|
No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
your last line contains a space so it's not considered as an extra line by the runbot
| self.env["account.move"].create( | ||
| [ | ||
| { | ||
| "partner_id": record.buyer_id.id, | ||
| "move_type": "out_invoice", | ||
| "invoice_line_ids": [ | ||
| Command.create( | ||
| {"name": record.name, "quantity": 1, "price_unit": record.selling_price * 0.06} | ||
| ), | ||
| Command.create({"name": "Administrative fees", "quantity": 1, "price_unit": 100}), | ||
| ], | ||
| } | ||
| ] | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| self.env["account.move"].create( | |
| [ | |
| { | |
| "partner_id": record.buyer_id.id, | |
| "move_type": "out_invoice", | |
| "invoice_line_ids": [ | |
| Command.create( | |
| {"name": record.name, "quantity": 1, "price_unit": record.selling_price * 0.06} | |
| ), | |
| Command.create({"name": "Administrative fees", "quantity": 1, "price_unit": 100}), | |
| ], | |
| } | |
| ] | |
| ) | |
| self.env["account.move"].create({ | |
| "partner_id": record.buyer_id.id, | |
| "move_type": "out_invoice", | |
| "invoice_line_ids": [ | |
| Command.create( | |
| {"name": record.name, "quantity": 1, "price_unit": record.selling_price * 0.06} | |
| ), | |
| Command.create({"name": "Administrative fees", "quantity": 1, "price_unit": 100}), | |
| ], | |
| }) |

No description provided.