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

[ADD] [15.0] base_warn_option #853

Merged
merged 1 commit into from
Apr 16, 2024
Merged

[ADD] [15.0] base_warn_option #853

merged 1 commit into from
Apr 16, 2024

Conversation

Shide
Copy link

@Shide Shide commented Apr 11, 2024 •
edited

This module extends the functionality of blocking messages to support more consistency
between messages and to allow you to set a blocking message quickly.

This is a PR of a series that depends on this one. Next PRs will add these modules on its respective repo:

sale_warn_option
purchase_warn_option
account_warn_option
stock_warn_option

MT-5193 @moduon @rafaelbn @yajo @EmilioPascual please review if you want :)

@yajo yajo added this to the 15.0 milestone Apr 11, 2024
@Shide Shide force-pushed the 15.0-base_warn_option-base_warn_option branch from 74a115f to 95c1aa8 Compare April 11, 2024 10:37
@Shide Shide requested a review from yajo April 11, 2024 11:24
rafaelbn
rafaelbn approved these changes Apr 11, 2024
View reviewed changes
Copy link
Member

@rafaelbn rafaelbn left a comment

Choose a reason for hiding this comment

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

I cannot test here.

In my side 👍🏼 base for the next

EmilioPascual
EmilioPascual approved these changes Apr 11, 2024
View reviewed changes
Copy link

@EmilioPascual EmilioPascual left a comment

Choose a reason for hiding this comment

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

LGTM

edlopen
edlopen approved these changes Apr 11, 2024
View reviewed changes
Copy link
Member

@edlopen edlopen left a comment

Choose a reason for hiding this comment

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

LGTM

base_warn_option/models/warn_option.py Show resolved Hide resolved
yajo
yajo requested changes Apr 12, 2024
View reviewed changes
Copy link
Member

@yajo yajo left a comment

Choose a reason for hiding this comment

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

This module looks good, but since it does "nothing", I'd like to review at least one of the upcoming extensions to understand how both relate before approving.

@Shide Shide force-pushed the 15.0-base_warn_option-base_warn_option branch from 95c1aa8 to fa66fce Compare April 15, 2024 09:06
@Shide
Copy link
Author

Shide commented Apr 15, 2024

Added a naviable button in configuration to view all the warning options

@Shide Shide requested a review from yajo April 15, 2024 11:29
This was referenced Apr 16, 2024
Merged
Open
yajo
yajo requested changes Apr 16, 2024
View reviewed changes
Copy link
Member

@yajo yajo left a comment

Choose a reason for hiding this comment

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

After reviewing dependant PRs OCA/sale-workflow#3076 and OCA/purchase-workflow#2240 I see clearly now how this module serves as a common base for those.

It looks OK but there's a missing requirement I just noticed before merging. Could you fix it please?

Thanks!

base_warn_option/__manifest__.py Outdated Show resolved Hide resolved
@Shide Shide force-pushed the 15.0-base_warn_option-base_warn_option branch from fa66fce to 020edd1 Compare April 16, 2024 10:43
@Shide Shide requested a review from yajo April 16, 2024 10:44
@yajo
Copy link
Member

yajo commented Apr 16, 2024

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 15.0-ocabot-merge-pr-853-by-yajo-bump-nobump, awaiting test results.

@Shide Shide mentioned this pull request Apr 16, 2024
Merged
@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). 🤖

@OCA-git-bot OCA-git-bot merged commit fbaee3a into OCA:15.0 Apr 16, 2024
5 checks passed
@OCA-git-bot
Copy link
Contributor

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

@Shide Shide deleted the 15.0-base_warn_option-base_warn_option branch April 16, 2024 11:01
@Shide Shide mentioned this pull request Apr 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yajo yajo yajo approved these changes

@rafaelbn rafaelbn rafaelbn approved these changes

@edlopen edlopen edlopen approved these changes

@EmilioPascual EmilioPascual EmilioPascual approved these changes

Assignees

@Shide Shide

Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
15.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Shide @yajo @OCA-git-bot @rafaelbn @edlopen @EmilioPascual