-
Notifications
You must be signed in to change notification settings - Fork 9.3k
[IMP] Attendances: Update overview section #13127
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
08689e8
to
5988cee
Compare
Hi @jero-odoo - this is ready for a peer review. |
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.
Hey @larm-odoo I left some comments, let me know if you have any questions!
I am not totally sure if this needed to be it's own doc, I think it made more sense as part of the original doc as is (especially since it is called "overview" I worry it might lead people to not view the setup doc first, which could cause some issues). I know the original doc was long but I don't think that was that much of an issue.
5988cee
to
01c4ac8
Compare
Hi @Felicious - this is ready for a final review! |
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.
Looks good so far, @larm-odoo !
A few thoughts to keep the article laser-focused on Attendances:
- Essentials overlap – Some view-switching and generic UI notes feel more “Odoo Essentials” than app-specific. Trimming those lines will tighten the intro and highlight what makes Attendances unique! 😊
- Doc placement – The overview section you’re adding fits better back in
attendances.rst
, keeping all core business relevance guidance in one spot. - Split for clarity & attendances doc length – Let’s spin Access Rights and Reporting section into their own docs (follow-up PRs). That keeps this page a crisp launchpad into the app’s capabilities.
I know you're planning a doc for all HR access rights, so we can put the Access Rights section there (: - Point value – With the scope narrowed, let's re-evaluate this PR to 2 or 3 points, depending on the % of lines changed in the PR (:
Happy to jump in with clarifications and wording tweaks if helpful. Thanks for driving the improvements for readability! 😊
01c4ac8
to
a6b051d
Compare
Hi @Felicious - I needed a day to absorb all of that =D I think the doc is so much stronger- thank you! I moved everything as suggested, and I will move the reporting doc to a new one. I was having issue,s since I will need to manually create a LOT of data to make the reporting work (the runbot has almost NOTHING, and one worker for one department, for 3 separate departments- it's a joke, and unusable!) so that will be written in a little while. So I will make a new doc - and yes, I will make that separate 'permissions' for HR only doc 0- but after my doc list. So this is ready for another look - let me know if I missed anything you suggested, or misinterpreted anything! |
Hi @Felicious - this is ready for another review! |
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.
Awesome work polishing this section, @larm-odoo! 😊
Love how you removed the Essentials content and added those Attendance-focused filters/group-bys in a clean table—super clear now. Everything looks good on my end. 🙌
a6b051d
to
b125f44
Compare
@samueljlieber - this is ready for a tech review =) |
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 @larm-odoo! Nice work on this PR!
I really like the way you explained how certain filters and grouping can be used, I think this is helpful :)
Im approving with a few suggestions for you to consider, and one mandatory update to the commit message!
Please update the commit message with the correct type (using git commit --amend
):
-[ADD] Attendances: Overview
+[IMP] Attendances: Update overview section
Thank you for your work!
..
@robodoo delegate=larm-odoo
b125f44
to
88dd7ac
Compare
88dd7ac
to
264a8aa
Compare
@robodoo r+ |
New doc moving and updating the Overview section to a standalone doc, and making all necessary updates.