-
Notifications
You must be signed in to change notification settings - Fork 9.2k
[ADD] Employees: Onboarding #13129
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] Employees: Onboarding #13129
Conversation
4b70d15
to
7dae822
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.
All set @larm-odoo! I left some comments (I would double check make review on some of the line breaks). Also I think there should be a mention as to what the users who are assigned the onboarding tasks see in their database, like the coaches and leads (see my note at the bottom). Let me know if you have any questions. Thanks!
to complete the onboarding plan. The steps appear in a list view, and each step displays the | ||
following information: | ||
|
||
- :guilabel:`Activity Type`: the specific activity for the step. The default options are :guilabel:`Email`, |
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.
Double check line length
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.
What's odd is that I did run make review- but it never catches this line, even though it looks like it's 110 long. I just checked again - an "no issues" is reported.
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.
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.
Seems like the sphinx-lint max-line-length check has an issue detecting lines ending with a comma!
However, this line is still detected by the VS Code highlight and indicated as over 100 chars by the ruler.
7dae822
to
5c0fd08
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.
Very detailed new doc, @larm-odoo ! Had some optional rewording suggestions for brevity for you to consider!
to complete the onboarding plan. The steps appear in a list view, and each step displays the | ||
following information: | ||
|
||
- :guilabel:`Activity Type`: the specific activity for the step. The default options are :guilabel:`Email`, |
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.
5c0fd08
to
1c76077
Compare
This is ready for a tech review @samueljlieber =) |
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 to me @larm-odoo, thank you for your work!
.....
@robodoo r+
No Onboarding doc currently exists.