Skip to content

[IMP] inventory: overhaul 3rd party shippers #8634

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
Closed
wants to merge 1 commit into from
Closed

[IMP] inventory: overhaul 3rd party shippers #8634

wants to merge 1 commit into from

Conversation

Felicious
Copy link
Contributor

Summary of changes

  1. Overhaul 8 year old 3rd party shipping carrier doc to 16.0
  2. Addresses @moch-odoo's suggestions to update shipping connector installation instructions
  3. Add a table of shipping methods and region availability in Odoo

FW port

Yes, all the way to master (:

Task

@Felicious Felicious requested a review from a team April 10, 2024 00:55
@Felicious Felicious self-assigned this Apr 10, 2024
@robodoo
Copy link
Collaborator

robodoo commented Apr 10, 2024

Pull request status dashboard.

brse-odoo
brse-odoo approved these changes Apr 10, 2024
View reviewed changes
Copy link
Contributor

@brse-odoo brse-odoo left a comment

Choose a reason for hiding this comment

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

@Felicious I've finished my review of this PR, great job! I'm approving with a few comments for you to accept/reject as you see fit. Let me know if you have any questions, thanks!

@Felicious Felicious mentioned this pull request Apr 10, 2024
Closed
@Felicious Felicious force-pushed the 16.0-inventory-third-party-shipper-feku branch 2 times, most recently from aab59bb to ed27fa1 Compare April 16, 2024 20:31
@Felicious
Copy link
Contributor Author

Hi @odoo/us-doc-review ! This doc is ready for review 😄

@Felicious Felicious requested a review from a team April 16, 2024 21:07
ksc-odoo
ksc-odoo suggested changes Apr 17, 2024
View reviewed changes
Copy link
Contributor

@ksc-odoo ksc-odoo left a comment

Choose a reason for hiding this comment

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

Hey @Felicious -- just finished my Final Review. Great work on this so far. I think we're just about at the finish line. Once you review my feedback, and make the necessary adjustments, feel free to tag me again for another quick look. Thanks! 👍

@Felicious Felicious mentioned this pull request Apr 17, 2024
Closed
@Felicious Felicious force-pushed the 16.0-inventory-third-party-shipper-feku branch from 6e6b9e9 to 7f253ff Compare April 18, 2024 02:12
@Felicious Felicious requested a review from ksc-odoo April 18, 2024 02:12
@Felicious
Copy link
Contributor Author

Thank you for the thorough, initial review, @ksc-odoo! I've addressed your comments and the doc is so much stronger now! Whenever you have a chance, I'd appreciate the second review!

ksc-odoo
ksc-odoo approved these changes Apr 18, 2024
View reviewed changes
Copy link
Contributor

@ksc-odoo ksc-odoo left a comment

Choose a reason for hiding this comment

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

Okie dokie, @Felicious -- after you address the few comments I've left, this should be ready to be tagged for Tech Review. Approving now! 👍

@Felicious Felicious force-pushed the 16.0-inventory-third-party-shipper-feku branch from 87dd60c to 163883d Compare April 18, 2024 21:47
@Felicious Felicious requested a review from samueljlieber April 18, 2024 21:47
@Felicious
Copy link
Contributor Author

Felicious commented Apr 18, 2024 •
edited
Loading

Hello @samueljlieber ! This PR is ready for your technical review 😄 Thank you, as always!

Also, I talked to Zac about this, and since this doc was 8 years old, and not much was still usable, this can be counted as 5 pts, not 3.

@Felicious Felicious added the 5 label Apr 18, 2024
samueljlieber
samueljlieber requested changes Apr 19, 2024
View reviewed changes
Copy link
Contributor

@samueljlieber samueljlieber left a comment

Choose a reason for hiding this comment

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

Hi @Felicious! Nice work on this fully improved Inventory doc. This PR is looking great, I only have a few small corrections and suggestions, please see below. Tag me for one more quick look after these are addressed, and let me know your thoughts on the list capitalization. Thank you!

...s/inventory_and_mrp/inventory/shipping_receiving/setup_configuration/third_party_shipper.rst Outdated
Comment on lines 239 to 241
.. image:: ../advanced_operations_shipping/invoicing/add-a-shipping-method.png
:align: center
:alt: Show the "Get rate" button in the "Add a shipping method" pop-up.
Copy link
Contributor

Choose a reason for hiding this comment

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

While using an image in another directory is technically sound– I don't recommend it since future modifications to the invoicing/ folder could affect this documentation and it goes against the media folder convention.

I suggest either adding the image to this document's media folder, referencing the section in a ref or seealso, or just removing the image entirely :)

(same goes for any referencing images outside of this documents media folder please)

Suggested change
.. image:: ../advanced_operations_shipping/invoicing/add-a-shipping-method.png
:align: center
:alt: Show the "Get rate" button in the "Add a shipping method" pop-up.
.. image:: ../advanced_operations_shipping/invoicing/add-a-shipping-method.png
:align: center
:alt: Show the "Get rate" button in the "Add a shipping method" pop-up.

@Felicious @brse-odoo
@ksc-odoo @samueljlieber
[IMP] inventory: overhaul 3rd party shippers
db9991d 
Co-authored-by: brse-odoo <brse@odoo.com>
Co-authored-by: ksc-odoo <73958186+ksc-odoo@users.noreply.github.com>
Co-authored-by: Sam Lieber (sali) <36018073+samueljlieber@users.noreply.github.com>
@Felicious Felicious force-pushed the 16.0-inventory-third-party-shipper-feku branch from 7afc52d to db9991d Compare April 19, 2024 17:19
@Felicious
Copy link
Contributor Author

Hi @samueljlieber ! Thank you so much for reminding me about the media file convention -- totally forgot about to reference it. Appreciate your attention to detail, as always! 😁

I've addressed all your comments, and the PR's ready for a second review

@Felicious Felicious requested a review from samueljlieber April 19, 2024 17:22
samueljlieber
samueljlieber approved these changes Apr 19, 2024
View reviewed changes
Copy link
Contributor

@samueljlieber samueljlieber left a comment

Choose a reason for hiding this comment

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

Nice job @Felicious, your changes look good to me! Nice job :)
.....
@robodoo r+

robodoo pushed a commit that referenced this pull request Apr 19, 2024
@Felicious @brse-odoo
@ksc-odoo @samueljlieber
[IMP] inventory: overhaul 3rd party shippers
4d7926f 
closes #8634

Signed-off-by: Samuel Lieber (sali) <sali@odoo.com>
Co-authored-by: brse-odoo <brse@odoo.com>
Co-authored-by: ksc-odoo <73958186+ksc-odoo@users.noreply.github.com>
Co-authored-by: Sam Lieber (sali) <36018073+samueljlieber@users.noreply.github.com>
@robodoo robodoo closed this Apr 19, 2024
This was referenced Apr 19, 2024
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@StraubCreative StraubCreative mentioned this pull request May 21, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samueljlieber samueljlieber samueljlieber approved these changes

+2 more reviewers

@ksc-odoo ksc-odoo ksc-odoo approved these changes

@brse-odoo brse-odoo brse-odoo approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@Felicious Felicious

Labels
5
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Felicious @robodoo @samueljlieber @ksc-odoo @brse-odoo