[IMP] Payroll: add necessary contract modules #9314
Conversation
|
Hi @hojo-odoo - this is ready for a peer review. Please note, I only added an important block in the Contract Details tab section of the doc, and I updated issues that appeared on the Linter (not using "in order to" and fixed all instances of "Kanban"). |
larm-odoo
force-pushed
the
15.0-payroll-adding-modue-info-to-contracts-larm
branch
2 times, most recently
from
7f162a1
to
133ec87
Compare
|
Hi @ksc-odoo - this is ready for a final review! This is similar to the other doc you reviewed- adding the same info to this doc, as it is necessary here as well. |
There was a problem hiding this comment.
Hey, @larm-odoo -- just finished my Final Review. Once you address the comments I've left, and implement the necessary adjustments, please tag me again for another look.
Also...
There are other formatting errors in lines 32-84, which are greyed out at the moment...the need for formatting corrections are pretty abundant in that section, so if you'd like to make another PR in the future to apply those adjustments, that would probably benefit the documentation a great deal.
👍
larm-odoo
force-pushed
the
15.0-payroll-adding-modue-info-to-contracts-larm
branch
from
133ec87
to
00382f9
Compare
|
Thank you @ksc-odoo! I decided to just go ahead and make those formatting edits, so I made sure everything after a colon was lower case, and I added the new icons as well. I think that's all the formatting issues that were from our new rules. I also made all the requested edits and updates. Ready for another look! |
There was a problem hiding this comment.
hey @larm-odoo --- almost there. please address the comments I've left, implement the necessary adjustments, and tag again when you think it's ready for another look. Thanks! 👍
larm-odoo
force-pushed
the
15.0-payroll-adding-modue-info-to-contracts-larm
branch
from
00382f9
to
8df7d27
Compare
|
Hi @ksc-odoo - thank you again for your very thorough review. I want to take a moment to apologize, because I honestly couldn't understand why my info was off or wrong- and you had so many correctoins to do. This PR was written to only add the needed module info (it was requested ot be added from a product owner- I know you saw the other docs I added this to, because they all had the same info in them). My intention was just to add that small part to this doc- a simple 2 point doc. So I did not go in and do all the updates as I normally would. I feel I should have been much more clear on that- because I could have done the module doc (for 2 pts) and then a 3 point doc to update the instrucitons (I al slowly learning how to gamify with points- it's not in my nature, so this is not easy for me, lol). Also, when I originally wrote the contracts doc, it was for 14- then they were ported up to master. This year, I decided to do all the 17 updates FIRST, then go back and do 16 updates. Only after that would I do any 15 updates, since that version was going to go away soon, and most likely not beofre I did all the updates for the newer 2 versions. So I just wanted to explain myself because I felt SO BAD that you had all these comments- and I honestly didn't know why things/images looked differet than the runbots I had originaly used- since I was only focusing on adding that admonition on the module. So, thank you for being so thorough- and please accept both my explanation and aplogy for all the work you did editing this doc- it was not my intention to have you do so much work! |
|
Oh, and this is ready for another look @ksc-odoo =D |
There was a problem hiding this comment.
Okie dokie, @larm-odoo -- just finished giving this another look -- and I'm approving now 👍 nice job on all the revisions. You're jusssst about at the finish line. Just have a handful of minor suggestions that require your attention. But, once you implement all the necessary changes, you can feel free to tag this for Tech Review. Thanks again for all your hard work on this one 👏
larm-odoo
force-pushed
the
15.0-payroll-adding-modue-info-to-contracts-larm
branch
from
8df7d27
to
7c24d19
Compare
|
Hi @samueljlieber - this is ready for a tech review. Thanks! |
There was a problem hiding this comment.
This looks great to me @larm-odoo, thanks for your hard work on this improvement to Contracts!
...
@robodoo r+
closes #9314 Signed-off-by: Samuel Lieber (sali) <sali@odoo.com>
|
@larm-odoo @samueljlieber this pull request has forward-port PRs awaiting action (not merged or closed): |
|
@larm-odoo @samueljlieber this pull request has forward-port PRs awaiting action (not merged or closed): |
2 similar comments
|
@larm-odoo @samueljlieber this pull request has forward-port PRs awaiting action (not merged or closed): |
|
@larm-odoo @samueljlieber this pull request has forward-port PRs awaiting action (not merged or closed): |
|
@larm-odoo @samueljlieber this pull request has forward-port PRs awaiting action (not merged or closed): |
|
@larm-odoo @samueljlieber this pull request has forward-port PRs awaiting action (not merged or closed): |
2 similar comments
|
@larm-odoo @samueljlieber this pull request has forward-port PRs awaiting action (not merged or closed): |
|
@larm-odoo @samueljlieber this pull request has forward-port PRs awaiting action (not merged or closed): |
Updating this doc to add a necessary module that must be installed. This was requested on this project card.
This change will go form 15 up to master, as this infomration is necessary in all versions.