Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUpdated the leave request process documentation from a single form-based submission to a Git-based workflow. The new process involves creating a branch in the HR repository, editing a leave.yaml file with entries for scheduled or taken leave, creating a pull request for approval, and receiving automatic leave summary recording upon merge. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
williamrusdyputra
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/LEAVE_POLICY.md`:
- Around line 32-33: Add required blank lines before each fenced code block and
before the list under the "Automated Leave Summary" heading: insert a single
blank line between the "Example:" line and the following ```yaml fence, and
insert a blank line between the sentence "You can use ranges for multi-day
leave:" and the following ```yaml fence; similarly, add a blank line between
"Every day at midnight UTC, the system automatically:" and the start of the list
item "- Calculates your leave balance..." and apply the same blank-line fixes to
the other occurrences referenced (lines around 58-60 and 66-67) so every fenced
code block and list has a preceding blank line.
- Around line 26-46: The ordered list numbering in the docs block should use the
"one." style required by the linter; update the list items so every step uses
"1." and keep the content intact (items referencing leave.yaml,
daysOff.scheduled, and daysOff.taken should remain unchanged), i.e., change the
current leading numbers for steps 2–6 to "1." so the entire sequence is numbered
with repeated "1." entries.
- Line 46: The markdown line "6. **After merge** - your leave is recorded and
will appear in the automated summary" exceeds 80 chars and should be wrapped to
satisfy MD013; edit the "6. **After merge** ..." line in LEAVE_POLICY.md and
split it into two shorter lines or sentences (for example break after "After
merge" or before "and will appear") so each line is <=80 characters while
preserving the same wording and emphasis.
Time Submission Status
You can submit time with the command. Example: See available commands to help comply with our Guidelines. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/LEAVE_POLICY.md`:
- Line 73: Clarify the ambiguous "HR issue" phrase by updating the sentence
"Updates your HR issue with a detailed leave summary" to explicitly state where
to find or create the HR issue (for example: "Updates your HR issue (the ticket
in the HR ticketing system/People Ops portal or the 'hr-requests' Slack channel)
with a detailed leave summary"), so readers know which system or location to
use; edit the LEAVE_POLICY.md line containing "HR issue" accordingly.
- Around line 40-42: Replace the past example date "2026-01-15" in the scheduled
leave YAML example with a future date (e.g., update the value of the date field
in the example entry that currently has date: 2026-01-15) so the sample shows a
valid upcoming leave; ensure the format remains YYYY-MM-DD and the other fields
(type: vacation, notes: Family trip) are unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/LEAVE_POLICY.md`:
- Around line 67-68: The Markdown has a missing blank line before the fenced
code block starting with "```yaml" after the sentence "or for multi-day but
separated, you can simply add more items on 1 PR:", which triggers MD031; add a
single blank line between that sentence and the code fence so the code block is
separated by an empty line and the lint rule passes.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/CONTRIBUTING.md`:
- Around line 138-140: The MD013 line-length warning comes from the long inline
link to "GitHub's documentation on commit signature verification"; fix it by
either inserting a markdownlint disable comment immediately before that link
line (<!-- markdownlint-disable-next-line MD013 -->) or converting the inline
URL to a reference-style link (replace the long inline URL with a short
reference like [GitHub's documentation][gh-signature] and add the full URL at
the bottom as [gh-signature]:
https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
so the content containing "GitHub's documentation on commit signature
verification" no longer exceeds the configured line length.
tiffmyaaw
left a comment
tiffmyaaw
left a comment
There was a problem hiding this comment.
My only comment is about the PR, as per our guideline each PR should link to an issue, it doesn't make sense to create a PR without referencing it to an issue first.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/LEAVE_POLICY.md`:
- Line 31: The line "You can define which days you are requesting on the issue
description, even if the days are separated like 12th, 14th, etc." exceeds 80
characters—wrap it to multiple shorter lines (<=80 chars) in LEAVE_POLICY.md so
it fits the max line length; split the sentence at a natural break (for example
after "issue description,") or rewrite into two sentences preserving the meaning
(e.g., one line explaining that days can be listed and a second line with the
example "12th, 14th, etc.").
tiffmyaaw
left a comment
tiffmyaaw
left a comment
There was a problem hiding this comment.
This is so far so good in my opinion 😃
|
@markholdex could you check this doc, thank you |
zolotokrylin
left a comment
zolotokrylin
left a comment
There was a problem hiding this comment.
I am not rejecting, as it is not completely wrong, but:
- move the HOW instruction inside the wizard docs, similar to: https://wizard.holdex.io/docs
- in this README just leave a reference to the instructions
|
Yes, I will move it there when this is decided: to avoid re-writing in case of any change. |
|
@williamrusdyputra, this is not blocking: The mentioned issue is an independent product iteration and therefore not included in the current scope. |
There was a problem hiding this comment.
@williamrusdyputra should we just link to the official docs instead of maintaining 2 places? https://wizard.holdex.io/docs/leave-request
Otherwise I need to reject your PR because it mismatches our official docs.
yes, that's my plan also |
zolotokrylin
changed the title
|
@holdex pr submit-time 12m |
4 participants
resolves: https://github.com/holdex/wizard/issues/853
Summary by CodeRabbit
Release Notes