-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[IMP] Recruitment: adding new hiring flow doc #5994
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
835ce19 to
cec738d
Compare
|
This doc is ready for review! There is one issue, where this doc references another doc that is currently being reviewed/is not published yet. This is the error that appears in the above check. I need to have that dc referenced, so this will need to wait to be published until after the other doc is published. But it can certainly be reviewed for it's content! |
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.
Hey @larm-odoo , great work. I had a few suggestions based on testing. Also just a few notes on the flow. I think the main instructions for communication (email templates, setting meetings, etc) would probably make sense independent of the specific stages, since they have more universal application. Let me know if you have any questions, I was having some issues with runbot most of the day, so if you want to go over any of the testing I did, please let me know. Thanks!
cec738d to
c0951c3
Compare
|
Thanks @jero-odoo - all items changed! If you want to take another look, I retagged you. TY! |
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.
@larm-odoo looks good, just noticed a few issues but once those are fixed I think this is ready to go 👍
c0951c3 to
9631610
Compare
|
OK- changes made, ready for your approval @jero-odoo =) |
|
OK- those last changes have been made. There is still a build error because this must be published AFTER [ADD] Recruitment: add new job position #4350 is closed (because this doc links to that one). So the build error should disappear once that PR is merged. I am putting this note in here so I don't forget later! |
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.
@larm-odoo I can't tell if there is something wrong with how the changes are appearing on my end or if they didn't get added on the last commit but there are two errors that look like they didn't get resolved last time (really small things). Other than that, looks good!
9631610 to
1305525
Compare
|
Thanks @jero-odoo - I either didn't save my changes when I edited this, or I doubled up on a section of commands (I know I got confused and redid 2 lines but I thought it was ok clearly not!). I had a merge conflict but I think I resolved it with this one. @odoo/miscellaneous-doc-review this is ready for the second round review. Thanks! |
tiku-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.
Making comments on this doc in stages because it is quite large and I don't want to lose my progress. Here are the first 140 lines reviewed. I will let you know when I've completed the review so you can make edits.
tiku-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.
@larm-odoo -- 2nd batch of comments, still working on this doc. Will have more time next week
tiku-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.
@larm-odoo Another stack of comments for you, almost done. 👍
tiku-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.
I've finished my first review of this document. Good start....I've made comments in batches. Can you address my changes, and I will take another look? For the screenshots, if you manually reduce the size of the window and then take a screenshot, it will appear zoomed.... some of your screenshots have tiny text.
Also -- the date selector in Odoo should not be referred to as the calendar module-- it is not connected to the calendar app in Odoo.
Lastly-- smart buttons are only those buttons (not solid in color) at the top of a record. Not the menu buttons but below in the primary box of the record. There are quite a few mislabeled buttons in this doc, I've tried to note them all.
I found some functional questions, which are the big ones I'd like to check again.
Great start to a thorough doc. Looking forward to see this published.
👍 Tim
1c77dba to
72d08cd
Compare
|
Hi @tiku-odoo - wow that almost took me as long to address all your comments as it did for you put write them =). |
72d08cd to
e97d023
Compare
tiku-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.
I've requested a few of changes. After these are made you can send it to final review. I'm noticing spelling errors on changes I requested in the last round. Can you fix these and do a read through to ensure there aren't more.
When you refer to documentation avoid referring to "this documentation" and link the title of the doc without the custom text.
Quite a few build errors on this doc as well, might want to make sure these are addressed before it goes to Sam.
Thanks,
Tim
e97d023 to
a5d9d9d
Compare
|
Thanks @tiku-odoo for the second look- and catching more things and the other suggestions. I will be sure to use the document name instead of 'refer to this doc'. I originally didn't since the titles weren't 100% fitting in the flow of the sentence, so I went with the generic. But those are all changed. I also linked the 'these are the stages' to each of the stages, which I think is also a good improvement. All the errors Sam knows about, and they are the result of a code owner issue, and referencing docs that aren't published (Job Positions in recruitment), so I tink we are good. @samueljlieber, this is ready for you! |
|
Hi @larm-odoo it seems you have included a doc that seems unrelated to this PR: This is where most of your build errors are coming from for the /documentation ci checks. Can you please either remove Also take a look at |
a5d9d9d to
1b27fec
Compare
|
All checks have finally passed, @samueljlieber ! =) |
samueljlieber
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 @larm-odoo, awesome work with the content in this very large doc, it is looking pretty great! I have a few technical suggestions throughout the doc: alt tags that are repeating the previous paragraph, anchor tag conventions, and a few other points 🙂
I also did not see a final content review from @odoo/us-doc-review, I should have tagged this before I started my review. Please have this PR go for a final content review before tagging technical again 🙏
Also, please update the PR title and commit message to use the appropriate commit tag for this PR since this PR is not adding a doc, rather it is improving an existing doc (recruitment.rst) 🙂
content/applications/hr/recruitment/applicant-acknowledgement.png
Outdated
Show resolved
Hide resolved
55ccf1c to
d0a6bff
Compare
|
Hi @samueljlieber - thank you for the review! Now I understand how (and why) to use more unique/appropriate anchor tags. Also, when I wrote this doc it was new, but that 'new job' doc changed this rst file- so that it technically existed =D so that's why it was an ADD and not an IMP. Everything should be all set now! |
samueljlieber
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 @larm-odoo, approving this PR. Everything looks good to me, nice job! On to you @StraubCreative 👍
|
After merging this PR, rebase and merge #6206. |
d0a6bff to
869ed4a
Compare
StraubCreative
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.
@robodoo r+
closes #5994 Signed-off-by: Zachary Straub (zst) <[email protected]>
New doc to explain the actual process of hiring a candidate, step by step, following the kanban flow.