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

[14.0][IMP] base_tier_validation: Add validation_status field to tier.validation #730

Merged
merged 1 commit into from
Oct 23, 2023
Merged

[14.0][IMP] base_tier_validation: Add validation_status field to tier.validation #730

merged 1 commit into from
Oct 23, 2023

Conversation

victoralmau
Copy link
Member

Add validation_status field to tier.validation

@Tecnativa TT33369

@OCA-git-bot
Copy link
Contributor

Hi @LoisRForgeFlow,
some modules you are maintaining are being modified, check this out!

@victoralmau victoralmau mentioned this pull request Oct 17, 2023
Merged
1 task
LoisRForgeFlow
LoisRForgeFlow reviewed Oct 18, 2023
View reviewed changes
Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This opens a good debate, now that I see validation_status I like it better than the current validated and rejected booleans... however, I don't think it is worth to refactor in current stables versions. So let's try to make these coexist without creating a big mess. See my comment:

base_tier_validation/models/tier_validation.py Show resolved Hide resolved
@victoralmau victoralmau force-pushed the 14.0-imp-base_tier_validation-TT33369 branch from 8f38412 to d904158 Compare October 18, 2023 14:56
@pedrobaeza pedrobaeza added this to the 14.0 milestone Oct 19, 2023
LoisRForgeFlow
LoisRForgeFlow requested changes Oct 19, 2023
View reviewed changes
Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

while open thread is not solved

@victoralmau victoralmau force-pushed the 14.0-imp-base_tier_validation-TT33369 branch from d904158 to 0406176 Compare October 19, 2023 12:04
LoisRForgeFlow
LoisRForgeFlow approved these changes Oct 19, 2023
View reviewed changes
Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM

@victoralmau victoralmau force-pushed the 14.0-imp-base_tier_validation-TT33369 branch from 0406176 to 476b00d Compare October 19, 2023 14:54
@victoralmau victoralmau force-pushed the 14.0-imp-base_tier_validation-TT33369 branch from 476b00d to ef85985 Compare October 19, 2023 14:58
@bosd
Copy link
Contributor

bosd commented Oct 19, 2023

@victoralmau
Just asking, are these changes still compatible with base_tier_validation_waiting?

@victoralmau
Copy link
Member Author

@victoralmau Just asking, are these changes still compatible with base_tier_validation_waiting?

This new field does not change any existing behavior (#730 (comment)), it is only something informative that can be used in other modules.

@pedrobaeza
Copy link
Member

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-730-by-pedrobaeza-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 7351d7d into OCA:14.0 Oct 23, 2023
4 of 6 checks passed
@OCA-git-bot
Copy link
Contributor

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

@pedrobaeza pedrobaeza deleted the 14.0-imp-base_tier_validation-TT33369 branch October 23, 2023 11:46
@victoralmau victoralmau mentioned this pull request Oct 24, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LoisRForgeFlow LoisRForgeFlow LoisRForgeFlow approved these changes

@pedrobaeza pedrobaeza pedrobaeza approved these changes

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

Successfully merging this pull request may close these issues.
5 participants
@victoralmau @OCA-git-bot @bosd @pedrobaeza @LoisRForgeFlow