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.

Sign up for GitHub

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

Jump to bottom

[16.0][MIG] edi_sale_oca: Migration to 16.0 #31

Merged
merged 19 commits into from
Nov 21, 2023
Merged

[16.0][MIG] edi_sale_oca: Migration to 16.0 #31

merged 19 commits into from
Nov 21, 2023

Conversation

QuocDuong1306
Copy link
Contributor

@QuocDuong1306 QuocDuong1306 commented Oct 18, 2023 •
edited by yvaucher
Loading

@QuocDuong1306 QuocDuong1306 mentioned this pull request Oct 18, 2023
Closed
@simahawk
Copy link
Contributor

In OCA/edi#759, should we need to move listeners.py in edi_sale_ubl_oca to edi_sale_oca for common?
Because in the future, maybe we need more modules like edi_sale_ubl_oca (ex: edi_sale_edifact_oca,...)

I'd do it only if we think that setting the state is needed in all the other cases is needed.
Otherwise, what would you move? An empty listener for create and write?

What is your POV on this?

@QuocDuong1306
Copy link
Contributor Author

Yes @simahawk, you're right. We need to move when all standards need setting the state. So please ignore it and could you review this module? Thank a lot

@yvaucher
Copy link
Member

Please squash Simone's commits

fixup! Add edi_sale_oca

fixup! fixup! Add edi_sale_oca

into

Add edi_sale_oca

yvaucher
yvaucher reviewed Oct 19, 2023
View reviewed changes
Copy link
Member

@yvaucher yvaucher left a comment

Choose a reason for hiding this comment

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

Squash to do and test-requirements.txt to update

Otherwise LGTM

test-requirements.txt Outdated
@@ -1 +1,2 @@
odoo_test_helper
odoo-addon-sale_order_import @ git+https://github.com/OCA/edi.git@refs/pull/816/head#subdirectory=setup/sale_order_import
Copy link
Member

Choose a reason for hiding this comment

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

Can be removed as OCA/edi#816 was just merged

@yvaucher yvaucher mentioned this pull request Oct 19, 2023
Merged
@QuocDuong1306
Copy link
Contributor Author

QuocDuong1306 commented Oct 20, 2023 •
edited
Loading

Hi @yvaucher , I updated. When squashing fixup! fixup! Add edi_sale_oca into Add edi_sale_oca, it was conflicted. So I squash fixup! fixup! Add edi_sale_oca into edi_sale: clean file and change the tile commit to edi_sale: clean file and drop *type_auto dependency. Thanks

@simahawk
Copy link
Contributor

Hi @yvaucher , I updated. When squashing fixup! fixup! Add edi_sale_oca into Add edi_sale_oca, it was conflicted. So I squash fixup! fixup! Add edi_sale_oca into edi_sale: clean file and change the tile commit to edi_sale: clean file and drop *type_auto dependency. Thanks

In theory it should have been squashed on the original PR... 😉

@QuocDuong1306
Copy link
Contributor Author

I updated

@QuocDuong1306 QuocDuong1306 force-pushed the 16.0-mig-edi_sale_oca branch 2 times, most recently from 59aa04d to 120a7ac Compare October 25, 2023 05:16
simahawk
simahawk requested changes Oct 26, 2023
View reviewed changes
edi_sale_oca/components/process.py Outdated Show resolved Hide resolved
edi_sale_oca/components/process.py Outdated Show resolved Hide resolved
edi_sale_oca/readme/DESCRIPTION.rst Outdated Show resolved Hide resolved
edi_sale_oca/readme/DESCRIPTION.rst Show resolved Hide resolved
@simahawk simahawk mentioned this pull request Oct 26, 2023
Merged
@simahawk
Copy link
Contributor

/ocabot rebase

@OCA-git-bot
Copy link
Contributor

Congratulations, PR rebased to 16.0.

@OCA-git-bot OCA-git-bot force-pushed the 16.0-mig-edi_sale_oca branch from ff33c2d to 5d3eec8 Compare November 15, 2023 11:05
@simahawk
Copy link
Contributor

/ocabot merge nobump

@simahawk
Copy link
Contributor

ah well, the OCA infra is down now... won't get any merge

@nilshamerlinck
Copy link
Contributor

@simahawk looks like Alexandre fixed the server issue, so the gitbot should be back, would you mind issuing the command again?

@simahawk
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit 8b5f898 into OCA:16.0 Nov 21, 2023
6 checks passed
@OCA-git-bot
Copy link
Contributor

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

@yvaucher yvaucher yvaucher left review comments

@simahawk simahawk simahawk approved these changes

Assignees
No one assigned
Labels
merged 🎉
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@QuocDuong1306 @simahawk @yvaucher @OCA-git-bot @nilshamerlinck