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][ADD] stock_release_channel_plan_process_end_time #696

Merged
merged 2 commits into from
Apr 3, 2024
Merged

[16.0][ADD] stock_release_channel_plan_process_end_time #696

merged 2 commits into from
Apr 3, 2024

Conversation

jbaudoux
Copy link
Contributor

@jbaudoux jbaudoux commented Jul 28, 2023 •
edited
Loading

@jbaudoux jbaudoux changed the title [ADD] stock_release_channel_plan_process_end_time [16.0][ADD] stock_release_channel_plan_process_end_time Jul 28, 2023
@jbaudoux jbaudoux added this to the 16.0 milestone Jul 28, 2023
@jbaudoux jbaudoux force-pushed the 16-release_channel_plan_process_end_time branch 3 times, most recently from fd432d6 to 63813e2 Compare July 28, 2023 23:33
lmignon
lmignon suggested changes Aug 1, 2023
View reviewed changes
Copy link
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

Can you add in a specific commit a test-requirements.txt file with the dependent PR as it's done here 758b160

@jbaudoux jbaudoux force-pushed the 16-release_channel_plan_process_end_time branch 2 times, most recently from 288e47c to b9981c8 Compare August 2, 2023 06:09
@jbaudoux
Copy link
Contributor Author

jbaudoux commented Aug 2, 2023

Can you add in a specific commit a test-requirements.txt file with the dependent PR as it's done here 758b160

Done. The failing test is not on this PR but on #550

@jbaudoux jbaudoux force-pushed the 16-release_channel_plan_process_end_time branch from b9981c8 to 173810c Compare August 18, 2023 08:38
simahawk
simahawk reviewed Oct 6, 2023
View reviewed changes
stock_release_channel_plan_process_end_time/tests/test_stock_release_channel_plan.py Outdated


class TestStockReleaseChannelPlan(
test_stock_release_channel_plan.TestStockReleaseChannelPlan
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm I'm not in favor of inheriting full test classes as every test you add there will run twice here.
Also overriding the method and calling super seems weird...
Can't you add a mixin and reuse it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@grindtildeath
Copy link
Contributor

jbaudoux#2

@grindtildeath grindtildeath mentioned this pull request Jan 24, 2024
Merged
3 tasks
@jbaudoux jbaudoux force-pushed the 16-release_channel_plan_process_end_time branch from 86e2708 to c0d88b9 Compare March 27, 2024 10:17
@jbaudoux
Copy link
Contributor Author

rebased & dropped do not merge commit

@jbaudoux jbaudoux force-pushed the 16-release_channel_plan_process_end_time branch from c0d88b9 to ec8798a Compare March 27, 2024 10:30
@jbaudoux
[IMP] stock_release_channel_plan: tests
d5cdb24 
Extract setup in common for inheritance
@jbaudoux jbaudoux force-pushed the 16-release_channel_plan_process_end_time branch 3 times, most recently from f085e4a to a0af5f2 Compare March 27, 2024 10:37
@jbaudoux jbaudoux force-pushed the 16-release_channel_plan_process_end_time branch from a0af5f2 to a2d96a6 Compare March 27, 2024 11:04
@jbaudoux
Copy link
Contributor Author

Tests have been fixed. Can you update your review @lmignon @simahawk

cc @sebalix

sbejaoui
sbejaoui approved these changes Mar 29, 2024
View reviewed changes
Copy link
Contributor

@sbejaoui sbejaoui left a comment

Choose a reason for hiding this comment

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

code review + functional test

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

@jbaudoux
Copy link
Contributor Author

jbaudoux commented Apr 3, 2024

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-696-by-jbaudoux-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 66ac807 into OCA:16.0 Apr 3, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@simahawk simahawk simahawk left review comments

@lmignon lmignon lmignon approved these changes

@sbejaoui sbejaoui sbejaoui approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.
6 participants
@jbaudoux @grindtildeath @OCA-git-bot @simahawk @lmignon @sbejaoui