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

[15.0][MIG] module_analysis #2358

Merged
merged 17 commits into from Feb 14, 2023
Merged

[15.0][MIG] module_analysis #2358

merged 17 commits into from Feb 14, 2023

Conversation

ghost
Copy link

@ghost ghost commented Jun 10, 2022

I migrated module_analysis module from 13.0 version to 15.0 version.

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Hi. Thanks for porting this module ! Could you squach your commit once migration is done to make the diff review easier to do ?
Regards.

Copy link
Contributor

@ivantodorovich ivantodorovich left a comment

Choose a reason for hiding this comment

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

Minor comment, otherwise seems ok
Code review only

module_analysis/__manifest__.py Show resolved Hide resolved
@sebalix sebalix added this to the 15.0 milestone Aug 3, 2022
@sebalix sebalix changed the title 15.0 mig module_analysis [15.0][MIG] module_analysis Aug 3, 2022
Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Hi. What is the state of this PR ?

Thanks !

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

thanks for the cherry pick !

Copy link

@cyrilmanuel cyrilmanuel left a comment

Choose a reason for hiding this comment

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

LGTM thank you :)

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@simahawk
Copy link
Contributor

simahawk commented Nov 4, 2022

@juliette-blanc can you please rewrite the commit msg for the migration?

[IMP] module_analysis: black, isort, prettier should be [MIG] module_analysis: migrate to v15

@simahawk
Copy link
Contributor

simahawk commented Nov 4, 2022

/ocabot migration module_analysis

@OCA-git-bot OCA-git-bot mentioned this pull request Nov 4, 2022
54 tasks
@simahawk
Copy link
Contributor

simahawk commented Nov 4, 2022

Hi. Thanks for porting this module ! Could you squach your commit once migration is done to make the diff review easier to do ?
Regards.

@legalsylvain actually this is not desired IMO. The migration commit the the linting commit should stay by themselves to:

  1. ease fwd porting of the migration commit
  2. ease diff: you'll see only relevant changes from the migration itself

Also, talking about commit messages: would you mind to rewrite yours? Is completely cut...
Should be something like:

[FIX+IMP] module_analysis : remove analysis when updating modules

When updating modules, the analysis is partial and makes the update slower.
Instead, use cron task that is executed nightly to update analysis automatically

Not a blocker, but nice to have before we merge one of the PRs otherwise is going to become harder to port it (msg won't match if we rewrite in one branch only).

@rvalyi
Copy link
Member

rvalyi commented Jan 27, 2023

for the record this v14 migration PR has more commits #2085, I'm trying to get it merged...

legalsylvain and others added 14 commits February 7, 2023 10:05
fixup! [ADD] new module module_analysis

fixup! fixup! [ADD] new module module_analysis

fixup! fixup! fixup! [ADD] new module module_analysis

fixup! fixup! fixup! fixup! [ADD] new module module_analysis

IMP exception message

fixup! fixup! fixup! fixup! fixup! [ADD] new module module_analysis

[REF] remove use of cloc. use pygount librairy instead

fixup! [REF] remove use of cloc. use pygount librairy instead

fixup! fixup! [REF] remove use of cloc. use pygount librairy instead

Apply suggestions from code review

Co-Authored-By: David Beal <[email protected]>

Update module_analysis/views/menu.xml

Co-Authored-By: David Beal <[email protected]>

Update module_analysis/tests/test_module.py

Co-Authored-By: David Beal <[email protected]>

Update module_analysis/readme/CONFIGURE.rst

Co-Authored-By: David Beal <[email protected]>

[IMP] handle encoding

[UPD] Update module_analysis.pot

[UPD] README.rst

[UPD] README.rst
Currently translated at 100.0% (35 of 35 strings)

Translation: server-tools-12.0/server-tools-12.0-module_analysis
Translate-URL: https://translation.odoo-community.org/projects/server-tools-12-0/server-tools-12-0-module_analysis/zh_CN/

[UPD] README.rst
Currently translated at 100.0% (35 of 35 strings)

Translation: server-tools-13.0/server-tools-13.0-module_analysis
Translate-URL: https://translation.odoo-community.org/projects/server-tools-13-0/server-tools-13-0-module_analysis/it/
royle-vietnam and others added 3 commits February 7, 2023 10:05
[MIG] module_analysis: Migration to 15.0
…cause the analysis is partial (it also make the update slower) ; Add instead a cron task that is executed nightly to update analysis automatically
@sebalix
Copy link
Contributor

sebalix commented Feb 14, 2023

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 15.0-ocabot-merge-pr-2358-by-sebalix-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 8394752 into OCA:15.0 Feb 14, 2023
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 4627862. Thanks a lot for contributing to OCA. ❤️

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

Successfully merging this pull request may close these issues.