-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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] Payroll: Work entry analysis report #11063
Conversation
ce7c782
to
242406d
Compare
Hi @ksc-odoo - this is ready for a first-round review! Just an FYI, the only reason other docs were edited including this one, is I needed to change a custom anchor. I discovered one wasn't pointing to the right anchor, and it was the anchor I needed to use in THIS doc! Just wanted to explain why all those docs are here with one small edit in each! |
242406d
to
7d91eb1
Compare
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 -- just finished my Peer Review. Nice job. Just have a small handful of comments that require your attention. Once you implement the necessary changes, feel free to move this to Final Review. Thanks! 👍
7d91eb1
to
e5cb7c4
Compare
Hi @StraubCreative - KSC did the first review, so this is for you when you have a chance. Thanks! |
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
Well written PR, the instructions were super clear and easy to follow— good work!
I do have one functional issue that needs addressing, see below.
Please let me know asap when this is updated (we can chat about it too if you want), and I'll pass for merge after, thanks!
e5cb7c4
to
2a5bd80
Compare
OK @StraubCreative - I updated the note to cover some more possible issue, but I did double-check the information on several runbots, and it does work. You should be seeing all this by default, so let me know if you still don't and I'll try and see what's going on! |
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.
Thanks for validating @larm-odoo.
Approved, with the optional suggestion I left above if not addressed already.
@samueljlieber this is ready for you.
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 @larm-odoo! Nice work on this reporting doc! Approving with a quick technical fix. Thank you for your work!
.....
@robodoo delegate=larm-odoo
2a5bd80
to
a24240e
Compare
@robodoo r+ |
closes #11063 Signed-off-by: Lara Martini (larm) <[email protected]>
closes #11063 Signed-off-by: Lara Martini (larm) <[email protected]>
closes #11063 Signed-off-by: Lara Martini (larm) <[email protected]>
closes #11063 Signed-off-by: Lara Martini (larm) <[email protected]>
closes #11063 Signed-off-by: Lara Martini (larm) <[email protected]>
Adding the work entry analysis reporting doc as a separate stand-alone doc instead of combined in a reporting doc with all reports on it.
Original task card for this PR.