-
Notifications
You must be signed in to change notification settings - Fork 43
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
[ADD]car_rental: car rental #36
base: 17.0
Are you sure you want to change the base?
Conversation
95024e9
to
396353e
Compare
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.
Hi @kir-odoo
Thanks for this work!
Please find below a (small 👀) list of comments. Please apply to all files.
Since this is my first PR review in industry, there may be some comments that we need to discuss, so don't hesitate to reply if you don't agree.
Cheers
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.
Second review based on a functional testing. I'll add a few other remarks on the task.
car_rental/static/src/binary/ir_attachment/835-web.assets_frontend.css.map
Outdated
Show resolved
Hide resolved
Missing file: car_rental/static/description/icon.png |
182aaf2
to
faf8436
Compare
211067e
to
363cd93
Compare
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.
Hi @dhrs-odoo
Still a few things to improve here.
Most important is without sale.temporal.recurrence you can't have rental prices...
Also, could you make sure the runbot is green?
Cheers
car_rental/demo/crm_lead.xml
Outdated
<br /> | ||
]]> | ||
</field> | ||
<field name="email_normalized">[email protected]</field> |
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.
Better be cautious
<field name="email_normalized">megha@gmail.com</field> | |
<field name="email_normalized">megha@example.com</field> |
@@ -0,0 +1,33 @@ | |||
<?xml version='1.0' encoding='UTF-8'?> | |||
<odoo> |
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.
Do you need this file? There is no theme to apply...
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.
Yes we need this 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.
Unused?
bdbcc0d
to
7ce9e16
Compare
…able instead of purchased or sale
…oduct.template view
48908da
to
e5ed4a6
Compare
e5ed4a6
to
71d88ba
Compare
task-3460478