[REF] inventory: move removal strats into own folder #7530

Felicious · 2024-01-24T23:06:42Z

Move the removal strategy docs out of advanced operations and into its own directory.

robodoo · 2024-01-24T23:07:38Z

Pull request status dashboard.

StraubCreative · 2024-01-25T02:06:03Z

@samueljlieber can you run sanity checks here and ensure everything is proper before JCS's review?

StraubCreative

@Felicious comment below for your consideration.

content/applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies.rst

samueljlieber

Hi @Felicious, this is just a quick review on the restructure before passing to JCS! Coincidentally, I was just reading this documentation yesterday and thought that this restructure was needed, so great timing!

I agree with ZST and JCS that removal_strategies.rst can contain the contents of removal.rst:

removal_strategies.rst (removal.rst contents)
├── closest_location.rst
├── fefo.rst
├── fifo.rst
├── least_packages.rst
└── lifo.rst

You are also missing redirects for each of the moved files, please add them to the redirects/17.0.txt file.

Once these edits are made, tag me once more so I can review before I pass it along 🙂
Thank you!

samueljlieber

Hi @Felicious, thank you for implementing the changes! I have just a couple of structural fixes, please see below for reference. I will push up a commit with these changes so we can get this PR in structure review 🙂 I am approving my review of the structure, I will still need to do a full technical review.

samueljlieber · 2024-01-26T15:27:34Z

content/applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies.rst

@@ -1,7 +1,20 @@
+:nosearch:


No need to include the :nosearch: meta tag because removal_strategies.rst now has content that we want searchable (instead of only a TOC).

Suggested change

:nosearch:

samueljlieber · 2024-01-26T15:30:00Z

content/applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies.rst


 .. list-table::
   :header-rows: 1
   :stub-columns: 1


This is out of scope of this PR, but want to note that this table does not format well on mobile. Maybe in another [IMP] PR this could be adjusted? I could see a card layout being useful here, the Accounting and Invoicing doc is a good example.

samueljlieber · 2024-01-26T15:32:39Z

redirects/17.0.txt

+
+# removal strategies intro
+
+applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/removal.rst
+applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies.rst
+
+# fifo
+applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/fifo.rst
+applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/fifo.rst
+
+# lifo
+applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/lifo.rst
+applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/lifo.rst
+
+# fefo
+applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/fefo.rst
+applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/fefo.rst
+
+# closest locations
+applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/closest_location.rst
+applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/closest_location.rst
+
+# least packages
+applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/least_packages.rst
+applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/least_packages.rst


Each redirect should be on one line, grouped by the app/module, and a comment summarizing the redirect can be added at the end of the redirect line 🙂

Suggested change

# removal strategies intro

applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/removal.rst

applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies.rst

# fifo

applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/fifo.rst

applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/fifo.rst

# lifo

applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/lifo.rst

applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/lifo.rst

# fefo

applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/fefo.rst

applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/fefo.rst

# closest locations

applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/closest_location.rst

applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/closest_location.rst

# least packages

applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/least_packages.rst

applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/least_packages.rst

# inventory_and_mrp/inventory

applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/closest_location.rst applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/closest_location.rst # advanced_operations_warehouse/closest_location.rst --> removal_strategies/closest_location.rst

applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/fefo.rst applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/fefo.rst # advanced_operations_warehouse/fefo.rst --> removal_strategies/fefo.rst

applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/fifo.rst applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/fifo.rst # advanced_operations_warehouse/fifo.rst --> removal_strategies/fifo.rst

applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/least_packages.rst applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/least_packages.rst # advanced_operations_warehouse/least_packages.rst --> removal_strategies/least_packages.rst

applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/lifo.rst applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/lifo.rst # advanced_operations_warehouse/lifo.rst --> removal_strategies/lifo.rst

applications/inventory_and_mrp/inventory/warehouses_storage/advanced_operations_warehouse/removal.rst applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies.rst # advanced_operations_warehouse/removal_strategies.rst --> removal_strategies/removal_strategies.rst

Felicious · 2024-01-29T17:17:17Z

Hi @jcs-odoo ! This restructure PR in the inventory scope is ready for your review (: Appreciate your help with this!

jcs-odoo

Hi!

Nice work! :) I left a few really minor comments, but you can directly merge this once addressed.

@robodoo delegate+

Cheers!

jcs-odoo · 2024-02-01T16:23:33Z

content/applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies.rst

+.. toctree::
+   :titlesonly:
+
+   removal_strategies/fifo
+   removal_strategies/lifo
+   removal_strategies/fefo
+   removal_strategies/closest_location
+   removal_strategies/least_packages
+


minor suggestion: put the toctree at the end of the document, as it is usually what we do on pages that have both content to display and children pages.

jcs-odoo · 2024-02-01T16:27:20Z

...cations/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/least_packages.rst


 .. seealso::
-   - :doc:`Other removal strategies <removal>`
+   - :doc:`About removal strategies <../removal_strategies>`


Technically, changing the wording would be part of another commit since it could also be applied to older versions than 17.0. But this is such a minor change that you can disregard my comment. It's mostly to keep in mind that when we apply different types of changes, they may apply to different versions.

jcs-odoo · 2024-02-01T16:27:39Z

content/applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies/fefo.rst


 .. seealso::
-   :doc:`Other removal strategies <removal>`
+   :doc:`About removal strategies <../removal_strategies>`


same here as my other comment

redirects/17.0.txt

Felicious · 2024-02-01T19:11:19Z

@robodoo r+

robodoo · 2024-02-01T20:50:34Z

@Felicious staging failed: ci/runbot on 8aafad5ed7edc56dacfa9876aafebc03e66cd986 (view more at https://runbot.odoo.com/runbot/build/57528719)

StraubCreative · 2024-02-01T21:25:28Z

@robodoo retry

robodoo · 2024-02-01T23:36:36Z

@Felicious staging failed: ci/runbot on 93ebfd87f13ac41a4528e401af070906a1ec3ee8 (view more at https://runbot.odoo.com/runbot/build/57533470)

Co-authored-by: Jonathan <[email protected]>

Felicious · 2024-02-02T00:26:53Z

Rebase the PR again, as a sanity check and see if this will merge

Felicious · 2024-02-02T00:31:34Z

@robodoo retry

robodoo · 2024-02-02T00:32:18Z

I'm sorry, @Felicious: retry makes no sense when the PR is not in error.

Felicious · 2024-02-02T00:58:33Z

@robodoo r+

jcs-odoo · 2024-02-02T10:48:35Z

Hi! It seems you forgot to include some changes in the commit you pushed, at least the ones you marked as resolved for the redirects.txt file.

Cheers

Felicious · 2024-02-02T11:53:48Z

Hi @jcs-odoo ! Sorry for missing the changes. I took a look at the 17.0.text file and I couldn't find how it differs from your comment. I'm so sorry, but could you elaborate on what I missed?

jcs-odoo · 2024-02-02T12:00:18Z

Apologies, you are right @Felicious ! I opened the wrong tab and missed one of the forced-pushed changes.

Thanks again for your work

Felicious requested a review from a team January 24, 2024 23:06

Felicious self-assigned this Jan 24, 2024

C3POdoo requested review from a team January 24, 2024 23:08

Felicious force-pushed the 17.0-inventory-make-new-removal-strat-section branch from 19980bd to 3946ac0 Compare January 24, 2024 23:08

Felicious requested review from jcs-odoo and removed request for a team January 24, 2024 23:09

StraubCreative requested a review from samueljlieber January 25, 2024 02:05

StraubCreative reviewed Jan 25, 2024

View reviewed changes

content/applications/inventory_and_mrp/inventory/warehouses_storage/removal_strategies.rst Outdated Show resolved Hide resolved

samueljlieber requested changes Jan 25, 2024

View reviewed changes

Felicious force-pushed the 17.0-inventory-make-new-removal-strat-section branch from 3946ac0 to 72ce8b0 Compare January 26, 2024 03:13

samueljlieber approved these changes Jan 26, 2024

View reviewed changes

samueljlieber force-pushed the 17.0-inventory-make-new-removal-strat-section branch from 72ce8b0 to e78f093 Compare January 26, 2024 15:38

jcs-odoo requested changes Feb 1, 2024

View reviewed changes

Felicious force-pushed the 17.0-inventory-make-new-removal-strat-section branch 2 times, most recently from 611da68 to f13c86f Compare February 1, 2024 19:02

[REF] inventory: move removal strats into own folder

a2fc9c6

Co-authored-by: Jonathan <[email protected]>

Felicious force-pushed the 17.0-inventory-make-new-removal-strat-section branch from f13c86f to a2fc9c6 Compare February 2, 2024 00:26

robodoo temporarily deployed to merge February 2, 2024 03:29 Inactive

robodoo closed this in 81509af Feb 2, 2024

This was referenced Feb 2, 2024

[FW][REF] inventory: move removal strats into own folder #7616

Closed

[FW][REF] inventory: move removal strats into own folder #7617

Closed

[REF] inventory: move removal strats into own folder #7530

[REF] inventory: move removal strats into own folder #7530

Uh oh!

Conversation

Felicious commented Jan 24, 2024

Uh oh!

robodoo commented Jan 24, 2024

Uh oh!

StraubCreative commented Jan 25, 2024

Uh oh!

StraubCreative left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

samueljlieber left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samueljlieber left a comment

Choose a reason for hiding this comment

Uh oh!

samueljlieber Jan 26, 2024

Choose a reason for hiding this comment

Uh oh!

samueljlieber Jan 26, 2024

Choose a reason for hiding this comment

Uh oh!

samueljlieber Jan 26, 2024

Choose a reason for hiding this comment

Uh oh!

Felicious commented Jan 29, 2024

Uh oh!

jcs-odoo left a comment

Choose a reason for hiding this comment

Uh oh!

jcs-odoo Feb 1, 2024

Choose a reason for hiding this comment

Uh oh!

jcs-odoo Feb 1, 2024

Choose a reason for hiding this comment

Uh oh!

jcs-odoo Feb 1, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Felicious commented Feb 1, 2024

Uh oh!

robodoo commented Feb 1, 2024

Uh oh!

StraubCreative commented Feb 1, 2024

Uh oh!

robodoo commented Feb 1, 2024

Uh oh!

Felicious commented Feb 2, 2024

Uh oh!

Felicious commented Feb 2, 2024

Uh oh!

robodoo commented Feb 2, 2024

Uh oh!

Felicious commented Feb 2, 2024

Uh oh!

jcs-odoo commented Feb 2, 2024

Uh oh!

Felicious commented Feb 2, 2024

Uh oh!

jcs-odoo commented Feb 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

samueljlieber left a comment •

edited

Loading