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

[15.0][MIG] product_sold_by_delivery_week: Migration to 15.0 #159

Merged
merged 9 commits into from
Dec 6, 2022
Merged

[15.0][MIG] product_sold_by_delivery_week: Migration to 15.0 #159

merged 9 commits into from
Dec 6, 2022

Conversation

cesar-tecnativa
Copy link

@Tecnativa TT36501

@cesar-tecnativa
Copy link
Author

@pedrobaeza please review

@cesar-tecnativa
Copy link
Author

@carlosdauden please review

@cesar-tecnativa cesar-tecnativa mentioned this pull request Jul 6, 2022
Closed
@pedrobaeza
Copy link
Member

/ocabot migration product_sold_by_delivery_week

@OCA-git-bot OCA-git-bot added this to the 15.0 milestone Jul 6, 2022
@OCA-git-bot OCA-git-bot mentioned this pull request Jul 6, 2022
Closed
16 tasks
pedrobaeza
pedrobaeza reviewed Jul 6, 2022
View reviewed changes
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

There are no unit tests, so it's difficult to know if everything goes right.

@cesar-tecnativa cesar-tecnativa force-pushed the 15.0-imp-product_sold_by_delivery_week branch from 3ea0e47 to 532e94c Compare July 6, 2022 09:28
@cesar-tecnativa
Copy link
Author

There are no unit tests, so it's difficult to know if everything goes right.

Added three tests

@cesar-tecnativa cesar-tecnativa force-pushed the 15.0-imp-product_sold_by_delivery_week branch 2 times, most recently from 490d487 to e3a6ee9 Compare July 6, 2022 09:48
@cesar-tecnativa
Copy link
Author

@LudLaf could you review this?

@pedrobaeza
Copy link
Member

@LudLaf instead of editing from GitHub interface, you have to do it on your local and commit only once following commit message guidelines.

@cesar-tecnativa cesar-tecnativa force-pushed the 15.0-imp-product_sold_by_delivery_week branch from 757028d to f3a564d Compare July 12, 2022 05:21
@LudLaf LudLaf force-pushed the 15.0-imp-product_sold_by_delivery_week branch from f3a564d to acedda8 Compare July 12, 2022 18:24
@LudLaf
Copy link

LudLaf commented Jul 12, 2022

@LudLaf instead of editing from GitHub interface, you have to do it on your local and commit only once following commit message guidelines.

Changes now made in local and pushed from there.

@cesar-tecnativa cesar-tecnativa force-pushed the 15.0-imp-product_sold_by_delivery_week branch from acedda8 to 661ba40 Compare July 14, 2022 00:29
@cesar-tecnativa
Copy link
Author

All the commits are correctly done now.

@cesar-tecnativa cesar-tecnativa force-pushed the 15.0-imp-product_sold_by_delivery_week branch from 661ba40 to 53fd708 Compare July 14, 2022 00:38
@cesar-tecnativa cesar-tecnativa force-pushed the 15.0-imp-product_sold_by_delivery_week branch from 53fd708 to fb8e9f7 Compare July 26, 2022 08:12
@cesar-tecnativa
Copy link
Author

@LudLaf could you review this?

pedrobaeza
pedrobaeza reviewed Jul 28, 2022
View reviewed changes
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

It seems there's still a lot of untested code.

product_sold_by_delivery_week/readme/CONFIGURE.rst Outdated

- For the sold char: `product_sold_by_delivery_week.sold_char`
- For the not sold: `product_sold_by_delivery_week.not_sold_char`
- For weeks length: `product_sold_by_delivery_week.weeks_to_consider`
- For weeks length: `product_sold_by_delivery_week.weeks_to_consider`
Copy link
Member

Choose a reason for hiding this comment

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

Incorrect extra indentation level. This goes in the first indentation.

product_sold_by_delivery_week/tests/test_product_sold_by_delivery_week.py Outdated
def setUpClass(cls):
super().setUpClass()
cls.env = cls.env(context=dict(cls.env.context, tracking_disable=True))

Copy link
Member

Choose a reason for hiding this comment

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

Remove empty lines inside a method.

product_sold_by_delivery_week/tests/test_product_sold_by_delivery_week.py Outdated
super().setUpClass()
cls.env = cls.env(context=dict(cls.env.context, tracking_disable=True))

cls.partner = cls.env.ref("base.res_partner_12")
Copy link
Member

Choose a reason for hiding this comment

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

Don't use demo data.

product_sold_by_delivery_week/tests/test_product_sold_by_delivery_week.py Outdated
Comment on lines 67 to 69
"""
Test a SO with a product on delivery.
"""
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""
Test a SO with a product on delivery.
"""
"""Test a SO with a product on delivery."""

@sergio-teruel
Copy link
Contributor

ping @chienandalu

@rousseldenis
Copy link
Contributor

@cesar-tecnativa Could you attend @pedrobaeza comments?

@pedrobaeza
Copy link
Member

César is no longer working on Odoo world, so another person will attend it.

@sergio-teruel sergio-teruel force-pushed the 15.0-imp-product_sold_by_delivery_week branch 2 times, most recently from a141777 to 613fb3e Compare December 1, 2022 20:40
@sergio-teruel
Copy link
Contributor

@pedrobaeza Changes done!!!
ping @CarlosRoca13

@sergio-teruel sergio-teruel force-pushed the 15.0-imp-product_sold_by_delivery_week branch from 613fb3e to 6eda7ac Compare December 1, 2022 20:51
@carlosdauden
Copy link
Contributor

We should supersede and add changes on #176

@CarlosRoca13
Copy link
Contributor

I will add the commits when this PR #196 will be marged to fix the CI too

@CarlosRoca13 CarlosRoca13 force-pushed the 15.0-imp-product_sold_by_delivery_week branch from 6eda7ac to 072689b Compare December 5, 2022 12:44
@CarlosRoca13 CarlosRoca13 force-pushed the 15.0-imp-product_sold_by_delivery_week branch from 072689b to 2d84c2c Compare December 5, 2022 12:46
@CarlosRoca13
Copy link
Contributor

Done @carlosdauden

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 15.0-ocabot-merge-pr-159-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit c17c599 into OCA:15.0 Dec 6, 2022
@OCA-git-bot
Copy link
Contributor

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

@pedrobaeza pedrobaeza pedrobaeza left review comments

@sergio-teruel sergio-teruel sergio-teruel approved these changes

@carlosdauden carlosdauden carlosdauden approved these changes

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

Successfully merging this pull request may close these issues.
9 participants
@cesar-tecnativa @pedrobaeza @LudLaf @sergio-teruel @rousseldenis @carlosdauden @CarlosRoca13 @OCA-git-bot @chienandalu