-
Notifications
You must be signed in to change notification settings - Fork 10.8k
[ADD] rental: service products page #15560
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
Conversation
433fb9d to
c36ec27
Compare
jero-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.
Hi @meval1006, nice work on this one! This was a big doc to pull together with all the apps it integrates, very impressive! Also, nice job with the use case 👍
I left some comments, let me know if you have any questions, and let me know when you want me to take another look. Thanks!
c0d3943 to
a681287
Compare
jero-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.
Hi @meval1006 ! Nice work on these updates! Still had a few things to change (mostly small).
Also, when you are committing the changes, are you adding them to a batch or committing them individually? Both will work, but if you add them to a batch, you only have to do a single commit, which makes it easier when you are squashing.
(It looks like you didn't have an issues squashing the changes this time so if that works for you by all means! Just wanted to make sure you knew this was an option. Let me know if you want to do a screen share to go over it!)

1cc8279 to
3e084fa
Compare
|
@jero-odoo Hopefully final review for approval. Thank you for catching all my typos. It's embarrassing how many there are without the linter to help me identify them. |
jero-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.
Hi @meval1006 just the one note (and this is my fault for not catching it last time! sorry about that!)
Great work! And I understand, personally I think the courier font in VS code makes it harder to find typos :)
This is ready for final/tech review!
cd5707c to
7eca92c
Compare
Felicious
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.
Amazing work with this PR, @meval1006 !
You took a previously undocumented workflow and built clear, end-to-end guidance for integrating four apps. I especially liked how you called out the key steps in Sign and Pickup & Returns—service products follow a different path than physical goods, and your emphasis makes that distinction easy to follow. This doc is going to be genuinely helpful for customers. 😊
In my review, I’ve included a few style suggestions. Some are purely optional and reflect personal preference rather than the team style guide, so feel free to ignore those if they don’t match your vision. I’ve labeled which ones are optional.
Thanks as well for being so thorough in validating details with the SME. That level of accuracy makes the doc stronger and lets me focus my feedback on readability and beginner-friendliness.
@robodoo delegate+
cd29fec to
4e6f7f3
Compare
|
@robodoo r+ |

Added a new page under the Rental app documentation.