[ADD] Time Off - Time off types#17767
Open
larm-odoo wants to merge 1 commit into
Open
Conversation
Collaborator
|
|
Contributor
Author
|
Hi @huisit - this doc is ready for review! |
larm-odoo
force-pushed
the
19.0-time-off-new-types-doc-larm
branch
from
333a7f7 to
0136ef0
Compare
larm-odoo
force-pushed
the
19.0-time-off-new-types-doc-larm
branch
from
0136ef0 to
07bd894
Compare
huisit
reviewed
May 12, 2026
huisit
approved these changes
May 12, 2026
Contributor
huisit
left a comment
huisit
left a comment
There was a problem hiding this comment.
@larm-odoo ooh this one is satisfying, the parent article was SO long before! I just have a few suggestions:
- Remove top-level heading: It isn't necessary since there are no other headings on the same level. Removing it means the reader can see all the sections in the TOC immediately, similar to your New employees article! If you do, don't forget to update the heading delimiters.
- Consider swapping first sentence: IMO the first sentence of the second paragraph ("One of the first things ") feels like information you'd lead with right out the gate, since it explains why this article is first.
- Link to Timesheets article: It seems like the Timesheets section is no longer visible here in Time Off for 19.0, that's why you removed it right? I'm not sure how significant the use case is (maybe small if it was removed), but we could consider adding a
seealsoortiplinking to the Time off entries article, since the functionality still exists in the Timesheets app. - Stick to one tense: There are a few places where I suggested "if employees should be able to" instead of "are able to." I'm not strongly attached to either, but make sure you use the same tense in all instances!
And just to be safe since I don't see a note about version -- I'm noticing significant changes in 19.2 and 19.3, and one extra field in 19.1. Are you planning separate PRs for those? Otherwise this is great and my suggestions are trivial, approved!
larm-odoo
force-pushed
the
19.0-time-off-new-types-doc-larm
branch
from
07bd894 to
3593aa7
Compare
Contributor
Author
|
Hi @Felicious - this is ready for a final review! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Separating out this from the main Time Off doc, and doing all relevant updates. The form has changed - new sections/things moved/labels changed.
Original task card for this PR.