search

OCA / vertical-association

Odoo addons for membership related tasks
GNU Affero General Public License v3.0
38 stars 112 forks source link

[17.0][MIG] membership_extension: Migration to 17.0 #170

Closed RogerSans closed 2 months ago

RogerSans commented 3 months ago

Standard migration T-6475

@HaraldPanten @ValentinVinagre

RogerSans commented 3 months ago

Solved THNX

RogerSans commented 2 months ago

Code and functional review.

The migration seems good

However, can you remove the membership_variable_period module from this migration.

Migrations PR's should only migrate one module

@RogerSans

Changed. THNX

HaraldPanten commented 2 months ago

@RogerSans don't you still have changes in membership_variable_period module ? Check pre-commit commit as well and make sure you are not including anything from membership_variable_period_module.

THX

RogerSans commented 2 months ago

@HaraldPanten Yes, I have made the changes that Alberto requested. However, in the pre-commit fix there is a reference to this module since a problem with a false error given by flake8 in the lambda function had to be solved. If I have to do it in a different pr I will change it.

rafaelbn commented 2 months ago

/ocabot migration membership_extension

ValentinVinagre commented 2 months ago

@RogerSans please remove the "membership_variable_period" module.

rafaelbn commented 2 months ago

MT-7084 @moduon please @edlopen @fcvalgar review 😄 ❤️

RogerSans commented 2 months ago

@HaraldPanten @ValentinVinagre done

HaraldPanten commented 2 months ago

THX for the reviews, @fcvalgar and @edlopen 👍

I think that now that we have functional and technical reviews, this PR is ready to merge. Could you do that, @rafaelbn ?

rafaelbn commented 2 months ago

@ValentinVinagre do you agree to merge?

rafaelbn commented 2 months ago

/ocabot merge patch

OCA-git-bot commented 2 months ago

This PR looks fantastic, let's merge it! Prepared branch 17.0-ocabot-merge-pr-170-by-rafaelbn-bump-patch, awaiting test results.

OCA-git-bot commented 2 months ago

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