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

[MIG][14.0] account_edi: Migration script #3040

Merged
merged 1 commit into from
Jan 5, 2022

Conversation

tranngocson1996
Copy link

@tranngocson1996 tranngocson1996 commented Dec 31, 2021

This PR

Migration script

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.

Same remarks for the 3 others PRs.

@tranngocson1996 tranngocson1996 force-pushed the v14_mig_account_edi branch 2 times, most recently from 8efaae6 to c9373d2 Compare January 3, 2022 01:20
@tranngocson1996 tranngocson1996 changed the title [MIG][14.0] account_edi: nothing to do [MIG][14.0] account_edi: Migration script Jan 3, 2022
@tranngocson1996
Copy link
Author

@legalsylvain i fixed it. what do you think?

@tranngocson1996
Copy link
Author

@legalsylvain Have you checked it for me yet?
cc: @pedrobaeza

@tranngocson1996
Copy link
Author

@pedrobaeza done bro. Check it for me again. Thank

@pedrobaeza pedrobaeza added this to the 14.0 milestone Jan 4, 2022
@pedrobaeza
Copy link
Member

I'm seeing that this module is a new one, and it will be auto-installed, so no migration script will be executed, so we have to turn the approach: instead of this, you should pre-create and fill the column on account migration script, commenting that is done for this module, and this one will be Nothing to do again.

@tranngocson1996
Copy link
Author

I'm seeing that this module is a new one, and it will be auto-installed, so no migration script will be executed, so we have to turn the approach: instead of this, you should pre-create and fill the column on account migration script, commenting that is done for this module, and this one will be Nothing to do again.

@pedrobaeza ok i understand. I will edit it right away.

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Thanks

@tranngocson1996
Copy link
Author

@pedrobaeza thank you sir. I'm trying to migrate 13 - 14, please support me and forgive the mistakes

@pedrobaeza
Copy link
Member

@legalsylvain please check your review

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 pedro ! Indeed if the module was not installed, migration script will not ve executed. Nice catch.

/ocabot merge nobump

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

@tranngocson1996
Copy link
Author

Thank @legalsylvain @pedrobaeza

@pedrobaeza pedrobaeza merged commit 8c89125 into OCA:14.0 Jan 5, 2022
@pedrobaeza
Copy link
Member

@legalsylvain remember that the ocabot command doesn't serve in the approval comment, but in OU I prefer to manually merge, as there's no advantage in triggering another build.

@legalsylvain
Copy link
Contributor

@pedrobaeza : ocabot merge doesn't work with OU ? It was working before. Or I'm wrong ?

@pedrobaeza
Copy link
Member

I'm not saying that. I quote my text: there's no advantage in triggering another build.

@legalsylvain
Copy link
Contributor

OK. sorry I missread.

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.

5 participants