[MOV] Payroll: moved configuration doc to main payroll so there is le… #5341
Conversation
|
Hi @samueljlieber - I am tagging you for the technical review since this is just moving info/the doc from the sub-head to the head of the payroll folder. Since it doesn't need a review of the content, it just needs to be reviewed that the item has moved and I did it right. |
samueljlieber
left a comment
samueljlieber
left a comment
There was a problem hiding this comment.
Hi @larm-odoo nice job with the move! I have a few technical corrections below for you 🙂
For any files moved or renamed, a redirect needs to be added so that if anyone tries to access the old location they get automatically redirected to the new location without receiving a 404 error 😉 Please add the following to the 14.0.txt redirects file, following the instructions:
# applications/payroll
applications/hr/payroll/configuration.rst applications/hr/payroll.rst # hr/payroll/configuration -> hr/payroll
Also, please update the commit message to use the [MOV] tag for moving files - see the commit tag template for more info. You can use the command git commit --amend to modify a commit message, or change the text while you are squashing.
Please tag me for another look once these changes are made, thank you! 🙏
larm-odoo
changed the title
larm-odoo
force-pushed
the
14.0-payroll-change-toc-moving-docs-larm
branch
from
2d1a768 to
88363b8
Compare
|
OK @samueljlieber - I did the redirect, looks like it's working! |
samueljlieber
left a comment
samueljlieber
left a comment
There was a problem hiding this comment.
Hi @larm-odoo! Nice job with adding the redirect, looks good to me 🙂 I left one small change with the H1, please discuss or make that correction before tagging for final review. Approving now 👍
content/applications/hr/payroll.rst
Outdated
| ===================== | ||
| Payroll configuration | ||
| ===================== |
There was a problem hiding this comment.
I'm suggesting removing "configuration" from the H1 since this is now the top-level doc for the app– configuration is implied IMO.
| ===================== | |
| Payroll configuration | |
| ===================== | |
| ======= | |
| Payroll | |
| ======= |
There was a problem hiding this comment.
OMG its the simplest things that I don't see =D. Great catch/idea, @samueljlieber! On to final review with @StraubCreative now.
There was a problem hiding this comment.
@StraubCreative - User Docs Structure Review was automatically tagged in this PR, but I wasn't sure if they need to be or not. I didn't think so, so I removed them.
There was a problem hiding this comment.
@StraubCreative - User Docs Structure Review was automatically tagged in this PR, but I wasn't sure if they need to be or not. I didn't think so, so I removed them.
No indeed, I just had a quick look to the app-level page's tags and it's all good, and the redirections don't require changing a doc link in the apps so it's all good :)
Cheers!
There was a problem hiding this comment.
Yea whenever we make structural changes UDS will be notified automatically, @larm-odoo
It's a good idea anyway that we get their eyes on changes like this :)
Thanks for taking a peek @jcs-odoo 🍻
There was a problem hiding this comment.
Thank you everyone for the look at this!
larm-odoo
force-pushed
the
14.0-payroll-change-toc-moving-docs-larm
branch
from
88363b8 to
d6abbb7
Compare
There was a problem hiding this comment.
seems good @larm-odoo 👍
caught a typo in the redirect comment if you want to address that quick
either way we're good to merge when you're ready, thanks!
@robodoo delegate=larm-odoo
redirects/14.0.txt
Outdated
|
|
||
| # applications/hr | ||
|
|
||
| applications/hr/payroll/configuration.rst applications/hr/payroll.rst # applications/hr/payroll/configuration.rs --> applications/hr/payroll.rst |
There was a problem hiding this comment.
not necessary for redirect functionality, just caught a small typo
| applications/hr/payroll/configuration.rst applications/hr/payroll.rst # applications/hr/payroll/configuration.rs --> applications/hr/payroll.rst | |
| applications/hr/payroll/configuration.rst applications/hr/payroll.rst # applications/hr/payroll/configuration.rst --> applications/hr/payroll.rst |
There was a problem hiding this comment.
TY! Fixed and trying to merge this now.
…ss clicking for users
larm-odoo
force-pushed
the
14.0-payroll-change-toc-moving-docs-larm
branch
from
d6abbb7 to
7184967
Compare
|
@robodoo r+ |
…ss clicking for users closes #5341 Signed-off-by: Lara Martini (larm) <larm@odoo.com>
5 participants
Just moving the config doc so that it is one level closer - it is in the main payroll doc now.