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

[16.0] [MIG] product_product_template_navigation #1456

Merged

Conversation

renda-dev
Copy link
Contributor

@renda-dev renda-dev commented Nov 21, 2023

Migration from 14.0 to 16.0 of product_product_template_navigation

@renda-dev renda-dev force-pushed the 14.0-mig-product_product_template_navigation branch 2 times, most recently from d50b842 to dddcf7e Compare November 21, 2023 10:13
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 take this opportunity to rename the module's name, which does not correspond to the standard of other OCA modules?

the other module that adds a link between a model and another, are suffixed by _link. so product_product_template_link could be a better name.

See the current 60 OCA modules :

image

@renda-dev renda-dev force-pushed the 14.0-mig-product_product_template_navigation branch 2 times, most recently from 835f561 to 54cedfc Compare November 21, 2023 11:59
@renda-dev
Copy link
Contributor Author

Hi. Thanks for porting this module.

could you take this opportunity to rename the module's name, which does not correspond to the standard of other OCA modules?

the other module that adds a link between a model and another, are suffixed by _link. so product_product_template_link could be a better name.

See the current 60 OCA modules :

image

Done! Thanks for the quick feedback :)

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.

Diff review. Lgtm

Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

Functional is ok, but why are we using the "archive" icon?

image

I think we could re-use the "variants" odoo standard icon, even if the buttons are not really specular

image

@francesco-ooops
Copy link
Contributor

@OCA/product-maintainers could you please add this to #1157 specifying the new module name?

also: should the PR have this name or the new module's name?

@renda-dev renda-dev force-pushed the 14.0-mig-product_product_template_navigation branch 2 times, most recently from a6e594d to f67c035 Compare November 22, 2023 09:01
@renda-dev
Copy link
Contributor Author

I totally agree with you @francesco-ooops, thanks for the suggestion! :)

@aleuffre aleuffre force-pushed the 14.0-mig-product_product_template_navigation branch 2 times, most recently from 8f89a08 to 08646ac Compare November 24, 2023 14:41
@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). 🤖

@aleuffre aleuffre force-pushed the 14.0-mig-product_product_template_navigation branch from 08646ac to b48b25d Compare November 28, 2023 09:38
@dreispt
Copy link
Member

dreispt commented Dec 22, 2023

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-1456-by-dreispt-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Dec 22, 2023
Signed-off-by dreispt
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 16.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 16.0-ocabot-merge-pr-1456-by-dreispt-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Dec 22, 2023
Signed-off-by dreispt
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 16.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 16.0-ocabot-merge-pr-1456-by-dreispt-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 94aa4f2 into OCA:16.0 Dec 22, 2023
9 checks passed
@OCA-git-bot
Copy link
Contributor

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

@renda-dev renda-dev deleted the 14.0-mig-product_product_template_navigation branch December 22, 2023 11:27
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.

8 participants