Skip to content
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] Appraisals: Appraisal Analysis reporting #11037

Open
wants to merge 1 commit into
base: 17.0
Choose a base branch
from

Conversation

larm-odoo
Copy link
Contributor

Original reporting doc covered both reports. There is now a Skills Analysis PR being reviewed, and this PR is to make the Appraisal Analysis report. The original "reportng.rst" file is being deleted.​

Original task card for this PR.

@robodoo
Copy link
Collaborator

robodoo commented Sep 16, 2024

Pull request status dashboard

@larm-odoo larm-odoo requested a review from a team September 16, 2024 22:03
@larm-odoo
Copy link
Contributor Author

Hi @Felicious - this is ready for a review!

@C3POdoo C3POdoo requested a review from a team September 16, 2024 22:03
@larm-odoo larm-odoo added the 5 label Sep 17, 2024
Copy link
Contributor

@Felicious Felicious left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing job with this new reporting doc, @larm-odoo !

Your doc is concise and easy to read and follow. It's also well-researched and I can tell you put in effort to find these various groups and filters. I just had very minor rewording suggestions, and after that, feel free to move this onto final review.

Thank you for giving me the chance to review docs again ❤️

@larm-odoo larm-odoo force-pushed the 17.0-appraisals-new-appraisal-analysis-larm branch from da8e45a to e3d9466 Compare September 17, 2024 20:44
@larm-odoo larm-odoo requested a review from a team September 17, 2024 20:44
@larm-odoo
Copy link
Contributor Author

Hi @ksc-odoo - this is ready for a final review!

Copy link
Contributor

@ksc-odoo ksc-odoo left a 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 Final Review. Nice job! Only have a handful of minor comments/suggestions that require your attention. Once you address those, and implement the necessary changes, feel free to tag this for Tech Review. Thanks! 👍

content/applications/hr/appraisals/appraisal_analysis.rst Outdated Show resolved Hide resolved
content/applications/hr/appraisals/appraisal_analysis.rst Outdated Show resolved Hide resolved
content/applications/hr/appraisals/appraisal_analysis.rst Outdated Show resolved Hide resolved
content/applications/hr/appraisals/appraisal_analysis.rst Outdated Show resolved Hide resolved
content/applications/hr/appraisals/appraisal_analysis.rst Outdated Show resolved Hide resolved
content/applications/hr/appraisals/appraisal_analysis.rst Outdated Show resolved Hide resolved
content/applications/hr/appraisals/appraisal_analysis.rst Outdated Show resolved Hide resolved
content/applications/hr/appraisals/appraisal_analysis.rst Outdated Show resolved Hide resolved
content/applications/hr/appraisals/appraisal_analysis.rst Outdated Show resolved Hide resolved
@larm-odoo larm-odoo force-pushed the 17.0-appraisals-new-appraisal-analysis-larm branch from e3d9466 to 9561821 Compare September 18, 2024 16:48
@larm-odoo larm-odoo requested a review from a team September 18, 2024 16:48
@larm-odoo
Copy link
Contributor Author

Hi @jero-odoo - this is ready for a final review. Thanks!

Copy link
Contributor

@jero-odoo jero-odoo left a 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 on this doc! Overall, everything looked great but a few things need to be fixed before this can be merged.

-Please delete all of the unused images from the image folder (I believe there are five extras in there)

  • Double check that the image users-appraisals.png has been compressed through pngquant
    -There needs to be a redirect for the old "reporting" doc, it doesn't appear that there is. Can you please add it?
    Let me know if you have any questions, thank you!

@larm-odoo larm-odoo force-pushed the 17.0-appraisals-new-appraisal-analysis-larm branch from 9561821 to d71c876 Compare September 18, 2024 20:11
@larm-odoo
Copy link
Contributor Author

Thanks @jero-odoo - when I made the new file, I didn't realize the old images were in there, and I also forgot the redirect! Everything should be in order if you want to take another look.

@jero-odoo
Copy link
Contributor

Looks good @larm-odoo thanks!

Sending to @StraubCreative for merge
.....

@StraubCreative
Copy link
Contributor

@robodoo r+

robodoo pushed a commit that referenced this pull request Sep 19, 2024
closes #11037

Signed-off-by: Zachary Straub (zst) <[email protected]>
robodoo pushed a commit that referenced this pull request Sep 19, 2024
closes #11037

Signed-off-by: Zachary Straub (zst) <[email protected]>
@robodoo
Copy link
Collaborator

robodoo commented Sep 19, 2024

@larm-odoo @StraubCreative staging failed: ci/runbot on bee603a7671353e808ccb0250242e60be8ba71b6 (view more at https://runbot.odoo.com/runbot/build/68483212)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants