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

[17.0][REF] stock_analytic: Split test utilities into shareable base class #682

Merged
merged 1 commit into from
Sep 5, 2024
Merged

[17.0][REF] stock_analytic: Split test utilities into shareable base class #682

merged 1 commit into from
Sep 5, 2024

Conversation

aisopuro
Copy link
Contributor

Needed to also rename method starting with double-underscrore to avoid Python method name mangling.

Intended to fix #644

@aisopuro aisopuro mentioned this pull request Aug 19, 2024
Merged
rousseldenis
rousseldenis reviewed Aug 19, 2024
View reviewed changes
stock_analytic/tests/test_stock_picking.py
@@ -12,7 +12,7 @@
from odoo.tests.common import TransactionCase


class TestStockPicking(TransactionCase):
class CommonStockPicking(TransactionCase):
Copy link
Contributor

Choose a reason for hiding this comment

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

Please put it in a separate common.py file

Copy link
Member

Choose a reason for hiding this comment

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

Please don't. That makes the blame a hell. It's not technically needed anyway, and I doubt its usefulness as organizational element.

@pedrobaeza pedrobaeza added this to the 17.0 milestone Aug 19, 2024
@aisopuro
Copy link
Contributor Author

Please hold off on merging this for a few minutes, it seems this change did not actually fix the issue I was trying to fix.

@StefanRijnhart
Copy link
Member

Please change commit message to include the module name. [REF] stock_analytic: split test utilities into shareable base class

@aisopuro aisopuro force-pushed the stock-analytic-shared-test-setup branch from 45f1153 to 6926243 Compare September 5, 2024 15:53
@aisopuro
[REF] stock_analytic: split test utilities into shareable base class
8488449 
Needed to also rename method starting with double-underscrore to
avoid Python method name mangling.
@aisopuro aisopuro force-pushed the stock-analytic-shared-test-setup branch from 6926243 to 8488449 Compare September 5, 2024 15:55
@aisopuro
Copy link
Contributor Author

aisopuro commented Sep 5, 2024

Please change commit message to include the module name. [REF] stock_analytic: split test utilities into shareable base class

Sorry for the wait @StefanRijnhart , this should be fixed now, and this PR is good to review and merge on my part.

@StefanRijnhart StefanRijnhart changed the title [REF] Split test utilities into shareable base class [REF] stock_analytic: Split test utilities into shareable base class Sep 5, 2024
StefanRijnhart
StefanRijnhart approved these changes Sep 5, 2024
View reviewed changes
Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

Thanks!

@StefanRijnhart
Copy link
Member

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 17.0-ocabot-merge-pr-682-by-StefanRijnhart-bump-minor, awaiting test results.

@StefanRijnhart StefanRijnhart changed the title [REF] stock_analytic: Split test utilities into shareable base class [17.0][REF] stock_analytic: Split test utilities into shareable base class Sep 5, 2024
@OCA-git-bot OCA-git-bot merged commit 0d5dc92 into OCA:17.0 Sep 5, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

aisopuro added a commit to avoinsystems/account-analytic that referenced this pull request Sep 5, 2024
@aisopuro
[BETA] Merge in-progress PRs:
be8f71c 
* OCA#682
* OCA#644
* OCA#687
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rousseldenis rousseldenis rousseldenis left review comments

@StefanRijnhart StefanRijnhart StefanRijnhart approved these changes

@pedrobaeza pedrobaeza pedrobaeza approved these changes

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

Successfully merging this pull request may close these issues.
5 participants
@aisopuro @StefanRijnhart @OCA-git-bot @pedrobaeza @rousseldenis