Skip to content

Conversation

Felicious
Copy link
Contributor

@Felicious Felicious commented Mar 18, 2024

Summary of changes

  • Move all inventory adjustment docs into the "inventory management" folder, as per @cru-odoo's request.
  • Add redirects to moved files

Reviewers

  • Since this PR is just to move files, no need to review for content (: If you have any suggestions about what other docs could go in this new inventory adjustment section or the order of the docs, feel free to share here!

FWport

Yes, all the way to master !

@robodoo
Copy link
Collaborator

robodoo commented Mar 18, 2024

@Felicious Felicious marked this pull request as draft March 18, 2024 21:48
@Felicious Felicious force-pushed the 16.0-inventory-adjustment-section-feku branch from 3b31fb3 to e174737 Compare March 18, 2024 22:19
@Felicious Felicious marked this pull request as ready for review March 18, 2024 22:19
@C3POdoo C3POdoo requested review from a team March 18, 2024 22:20
@Felicious Felicious removed the request for review from a team March 18, 2024 22:23
@Felicious
Copy link
Contributor Author

@odoo/inventory-doc-review Hello! This PR requires a different kind of review this time. I didn't change any files-- only moved inventory adjustment docs into its own section, so you don't need to review for content. Please let me know if you have any suggestions for doc structure:

  • Name of the new section I'm making
  • Order of the docs
  • Order of the inventory adjustment sectrion within the Warehouse/Storage parent folder
  • Any thoughts are welcome!

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.

Hi @Felicious, I've finished my "review" of this restructure PR. I think it looks great! The docs that you placed under the new Inventory Adjustments section are the ones I would have picked too -- I don't think there's any others I'd place under there for now, after looking. Plus it opens up the possibility of adding new related docs under there in the future.

I think the names are fitting, and everything looks good to me. I'm approving this for the next round, if you need any help or more input let me know! Thanks 👍

@Felicious
Copy link
Contributor Author

Hi @ksc-odoo, @StraubCreative, and @jcs-odoo! I'm thinking of moving these inventory adjustment docs to their own section. What do you all think? (:

@Felicious Felicious requested a review from a team March 21, 2024 23:45
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.

@Felicious -- seems good to me 👍 feel free to tag this for Tech Review whenever you get a chance. Thanks!

@samueljlieber samueljlieber requested a review from a team March 26, 2024 13:39
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 job with this refactoring PR! I have a suggestion to further improve the structure. In this PR's current state, there is are duplicate "Inventory adjustments" because count_products.rst is titled "Inventory adjustments".

current-fs8

I am suggesting to move the content within count_products.rst to inventory_adjustment.rst TOC file.

proposed-fs8

Doing so will simplify the directory structure and reduce the amount of clicks it takes the user to access the content.

If you choose to implement this suggestion, the count_products.rst redirect will need to be updated, the inventory_adjustment.rst TOC file will need meta tag updates, and any :doc: references.

remove :no-search: and add :show-content: meta tag

Thank you and let me know your thoughts!

@jcs-odoo
Copy link
Contributor

Hi @ksc-odoo, @StraubCreative, and @jcs-odoo! I'm thinking of moving these inventory adjustment docs to their own section. What do you all think? (:

Hi :)

Good idea.
And I was about to suggest the same as @samueljlieber :) It's best to have content in the category pages. And even better if the content in it helps navigate to the sub-pages.

The commit message and PR title should rather have the tag MOV instead of REF.
REF is rather for changes in the code. For structural changes, MOV is the right one.

Cheers :)

@Felicious Felicious changed the title [REF] inventory: create inventory adj section [MOV] inventory: create inventory adj section Apr 2, 2024
@Felicious Felicious force-pushed the 16.0-inventory-adjustment-section-feku branch from e174737 to 8efb2b1 Compare April 3, 2024 00:47
@samueljlieber samueljlieber added the 2 label Apr 4, 2024
@Felicious Felicious force-pushed the 16.0-inventory-adjustment-section-feku branch from 8efb2b1 to 129e6aa Compare April 8, 2024 18:10
@Felicious Felicious changed the title [MOV] inventory: create inventory adj section [MOV] inventory: group inventory adjustment docs together Apr 8, 2024
@Felicious Felicious force-pushed the 16.0-inventory-adjustment-section-feku branch from 129e6aa to 8f836db Compare April 8, 2024 18:47
@Felicious Felicious requested a review from samueljlieber April 8, 2024 18:52
@Felicious
Copy link
Contributor Author

Hi @samueljlieber ! Thank you for the suggestions to improve the searchability of the docs during your first review. This PR is ready for you to take another look at (:

So, it turns out, I misunderstood @cru-odoo a little-- she meant that she wanted all of the inventory adjustments grouped together, but they don't need to have their own, named section. So, I made the necessary adjustments to the redirects.txt and folders. Let me know what you think! (:

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, glad you were able to work with CRU to revise this PR. Everything looks good except for two small things in the redirect file. Can you tag me for one more quick look after these are addressed, thank you!

applications/inventory_and_mrp/inventory/management/lots_serial_numbers.rst applications/inventory_and_mrp/inventory/product_management/product_tracking.rst # /inventory_and_mrp/inventory/management/lots_serial_numbers -> /inventory_and_mrp/inventory/product_management/product_tracking
applications/inventory_and_mrp/inventory/management/lots_serial_numbers/differences.rst applications/inventory_and_mrp/inventory/product_management/product_tracking/differences.rst # /inventory_and_mrp/inventory/management/lots_serial_numbers/* -> /inventory_and_mrp/inventory/product_management/product_tracking/*
applications/inventory_and_mrp/inventory/management/lots_serial_numbers/serial_numbers.rst applications/inventory_and_mrp/inventory/product_management/product_tracking/serial_numbers.rst # /inventory_and_mrp/inventory/management/lots_serial_numbers/* -> /inventory_and_mrp/inventory/product_management/product_tracking/*
applications/inventory_and_mrp/inventory/management/lots_serial_numbers/lots.rst applications/inventory_and_mrp/inventory/product_management/product_tracking/lots.rst # /inventory_and_mrp/inventory/management/lots_serial_numbers/* -> /inventory_and_mrp/inventory/product_management/product_tracking/*
applications/inventory_and_mrp/inventory/management/lots_serial_numbers/expiration_dates.rst applications/inventory_and_mrp/inventory/product_management/product_tracking/expiration_dates.rst # /inventory_and_mrp/inventory/management/lots_serial_numbers/* -> /inventory_and_mrp/inventory/product_management/product_tracking/*
applications/inventory_and_mrp/inventory/management/inventory_adjustments.rst applications/inventory_and_mrp/inventory/warehouses_storage/inventory_management/count_products.rst # /inventory_and_mrp/inventory/management/inventory_adjustments -> /inventory_and_mrp/inventory/warehouses_storage/inventory_management/count_products
applications/inventory_and_mrp/inventory/management/inventory_adjustments/count_products.rst applications/inventory_and_mrp/inventory/warehouses_storage/inventory_management/count_products.rst # /inventory_and_mrp/inventory/management/inventory_adjustments/count_products -> /inventory_and_mrp/inventory/warehouses_storage/inventory_management/*
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why you removed the existing redirect for count_products.rst , please revert this so if any links to the old file location exist, the user is properly redirected :)

applications/inventory_and_mrp/inventory/management/inventory_adjustments/count_products.rst applications/inventory_and_mrp/inventory/warehouses_storage/inventory_management/count_products.rst                              # /inventory_and_mrp/inventory/management/inventory_adjustments/count_products -> /inventory_and_mrp/inventory/warehouses_storage/inventory_management/*

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you SO much for catching this, Sam! I got confused by the long paths when moving things around. Definitely did NOT intend to remove this line. Thank you SOSOSO much, again!

@Felicious Felicious force-pushed the 16.0-inventory-adjustment-section-feku branch from ddeed08 to 65ef784 Compare April 9, 2024 21:28
@Felicious
Copy link
Contributor Author

Appreciate your help with this so much, @samueljlieber ! Thank you for catching that redirect link. I applied your suggestions and restored the previous link. It's ready for another look!

@Felicious Felicious requested a review from samueljlieber April 9, 2024 21:31
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, I am approving and pushing up a quick fix to the redirects.txt file, there was another small change you had on lines 66-67 that was causing a diff in those lines :).

I will merge shortly after pushing up 👍
..

Co-authored-by: Sam Lieber (sali) <36018073+samueljlieber@users.noreply.github.com>
@samueljlieber samueljlieber force-pushed the 16.0-inventory-adjustment-section-feku branch from 65ef784 to 646bf77 Compare April 9, 2024 22:10
@samueljlieber
Copy link
Contributor

@robodoo r+

@fw-bot
Copy link
Collaborator

fw-bot commented Apr 10, 2024

Forward-porting to 'master' (from #8629).

@fw-bot
Copy link
Collaborator

fw-bot commented Apr 10, 2024

Forward-porting to 'master' (from #8630).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants