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] mis_builder_demo: Migration to 16.0 #489

Closed
wants to merge 8 commits into from

Conversation

dzungtran89
Copy link
Contributor

@dzungtran89 dzungtran89 mentioned this pull request Mar 3, 2023
3 tasks
Copy link

@Camille0907 Camille0907 left a comment

Choose a reason for hiding this comment

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

Looks technically good :)

Copy link

@Camille0907 Camille0907 left a comment

Choose a reason for hiding this comment

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

I approved to fast, please check CI

@dzungtran89 dzungtran89 force-pushed the 16.0-mig-mis_builder_demo branch 2 times, most recently from 8edc57e to d76b96d Compare March 3, 2023 09:37
@dzungtran89
Copy link
Contributor Author

Hi @Camille0907 , thanks for your review

About the first failed check, it is expected (this is a safety check to make sure that we don't merge any PR who's dependencies are not merged yet)

@sbidoul
Copy link
Member

sbidoul commented Mar 12, 2023

@dzungtran89 thanks for this! Can you send this as a PR to acsone:16.0-mig-mis_builder-ape too? It will be easier to test everything together.

@dzungtran89
Copy link
Contributor Author

@dzungtran89 thanks for this! Can you send this as a PR to acsone:16.0-mig-mis_builder-ape too? It will be easier to test everything together.

@sbidoul the PR has been created to the target branch

@sbidoul sbidoul marked this pull request as draft March 14, 2023 17:54
@sbidoul
Copy link
Member

sbidoul commented Mar 24, 2023

mis_builder_demo is now included in #472. Without analytic because the query to get the correct amount is quite complex, and actually I don't see how to make it generic enough, for instance what to do when the analytic_distribution is null?

@sbidoul sbidoul closed this Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants