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] delivery_carrier_label_default: Migrate to version 14.0 #456

Closed
wants to merge 9 commits into from
Closed

[14.0] delivery_carrier_label_default: Migrate to version 14.0 #456

wants to merge 9 commits into from

Conversation

sonhd91
Copy link

@sonhd91 sonhd91 commented Jan 13, 2022 •
edited
Loading

Depend on PR#455
reuse _get_packages_from_picking for print default label for selected packs

@sonhd91 sonhd91 marked this pull request as ready for review January 13, 2022 03:42
@sonhd91 sonhd91 force-pushed the delivery_carrier_label_default branch from 580a47f to f560626 Compare January 17, 2022 11:32
@sonhd91
Copy link
Author

sonhd91 commented Jan 18, 2022

Working on red travis

@sonhd91 sonhd91 force-pushed the delivery_carrier_label_default branch 6 times, most recently from d3e6a9f to e781992 Compare January 19, 2022 10:40
@sonhd91 sonhd91 mentioned this pull request Jan 23, 2022
Closed
21 tasks
@hailangvn
Copy link
Contributor

Travis is now green.

@sebalix sebalix added this to the 14.0 milestone Mar 29, 2022
@sebalix
Copy link
Contributor

sebalix commented Mar 29, 2022

/ocabot migration delivery_carrier_label_default

@sonhd91 sonhd91 force-pushed the delivery_carrier_label_default branch from e781992 to e9fb080 Compare April 7, 2022 10:02
@github-actions
Copy link

github-actions bot commented Aug 7, 2022

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Aug 7, 2022
@github-actions github-actions bot closed this Sep 11, 2022
@sebalix sebalix reopened this Nov 15, 2022
@sebalix sebalix mentioned this pull request Nov 15, 2022
Closed
sebalix
sebalix reviewed Nov 15, 2022
View reviewed changes
Copy link
Contributor

@sebalix sebalix 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.

There was already a PR #396 open before this one (we missed it), and the migration commit is quite different, I can't say which one is the best.

I know we are using the current one in production for some months now, but I guess we have to check the other one (ping @sonhd91 ).
One thing we are missing here are the translations and README update commits, but it's minor, not sure we should worry about this.

@thomaspaulb
Copy link

Seems the main difference is that in #396 we are inheriting send_shipping instead of here alternative_send_shipping (need to dive deeper into it to judge this, @KKamaa could you comment on this?), as well as deleting some unit tests (which is bad, of course)

@sonhd91 red tests look easy to solve when the Development status of this one is set to Beta

yvaucher and others added 7 commits November 16, 2022 14:50
@sonhd91 sonhd91 force-pushed the delivery_carrier_label_default branch from e9fb080 to 6bec83f Compare November 16, 2022 08:36
@sonhd91
Copy link
Author

sonhd91 commented Nov 16, 2022 •
edited
Loading

Seems the main difference is that in #396 we are inheriting send_shipping instead of here alternative_send_shipping (need to dive deeper into it to judge this, @KKamaa could you comment on this?), as well as deleting some unit tests (which is bad, of course)

@sonhd91 red tests look easy to solve when the Development status of this one is set to Beta

Thank, I just update the branch

@sebalix
The alternative_send_shipping was used when we have picking(by default label would have no carrier) with pricelist module, since no carrier was choosen, so the exact_price and tracking_number should be False

@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Nov 20, 2022
chienandalu
chienandalu reviewed Nov 24, 2022
View reviewed changes
delivery_carrier_label_default/__manifest__.py
"license": "AGPL-3",
"installable": True,
"depends": [
"base_delivery_carrier_label",
Copy link
Member

Choose a reason for hiding this comment

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

Is this dependency really needed? Such a large one for a module that could stand alone...

@github-actions
Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Mar 26, 2023
@github-actions github-actions bot closed this Apr 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebalix sebalix sebalix left review comments

@chienandalu chienandalu chienandalu left review comments

@leemannd leemannd leemannd approved these changes

Assignees
No one assigned
Labels
migration stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Milestone
14.0
Development

Successfully merging this pull request may close these issues.
8 participants
@sonhd91 @hailangvn @sebalix @thomaspaulb @leemannd @chienandalu @yvaucher @sergio-teruel