Skip to content

Conversation

@BhaveshHeliconia
Copy link
Contributor

@BhaveshHeliconia BhaveshHeliconia commented Feb 11, 2025

Depends on #217

@BhaveshHeliconia BhaveshHeliconia mentioned this pull request Feb 11, 2025
17 tasks
@max3903 max3903 added this to the 18.0 milestone Apr 9, 2025
@bosd
Copy link
Contributor

bosd commented Apr 9, 2025

@BhaveshHeliconia Can you remove the test dependencies on this one?

@BhaveshHeliconia
Copy link
Contributor Author

@bosd, I removed test file.Please review the PR when you get a chance.

Copy link
Contributor

@bosd bosd left a comment

Choose a reason for hiding this comment

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

Functional review :+1
Please drop the last 2 commits. Test requirements does'nt have to be in the history.

Nit picking: Demo data should be moved to the demo folder.

Pre-approving.

Copy link

@Dranyel-Bosd Dranyel-Bosd left a comment

Choose a reason for hiding this comment

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

LGTM 💯

@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). 🤖

Copy link
Contributor

@bosd bosd left a comment

Choose a reason for hiding this comment

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

I think a missing piece in the brand repo is the ability to select a brand on a partner.
Then when a sale order is made, the quotation/SO is branded according to the brand set on the partner.
(Edit: Made #257)

This module add the fields the field to the partner module. But looks like it serves a different purpose.

IMO would love to have this module to be more neutral without the added (r) sign.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could inherit here from the mixing of the base module.
In that way the setting of the brand_use_level will be respected by this module.

@yvaucher
Copy link
Member

/ocabot migration partner_brand

@yvaucher
Copy link
Member

@BhaveshHeliconia Can you rebase? The requirement is not needed anymore.

@BhaveshHeliconia BhaveshHeliconia force-pushed the 18.0-mig-partner_brand branch from 8773765 to a7bbdde Compare July 29, 2025 11:28
@BhaveshHeliconia
Copy link
Contributor Author

@yvaucher : It's Done.

@yvaucher
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

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

OCA-git-bot added a commit that referenced this pull request Jul 29, 2025
Signed-off-by yvaucher
@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit f079098 into OCA:18.0 Jul 29, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 345fb43. 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.

9 participants