-
Notifications
You must be signed in to change notification settings - Fork 9k
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.
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
[IMP] PLM: Fix formating, add images, clarify #12725
base: 18.0
Are you sure you want to change the base?
Conversation
content/applications/inventory_and_mrp/plm/manage_changes/eco_type.rst
Outdated
Show resolved
Hide resolved
content/applications/inventory_and_mrp/plm/manage_changes/eco_type.rst
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the content looks good to me, @slinkous ! I'm just requesting changes so I can double-check the links after you take a look (:
content/applications/inventory_and_mrp/plm/manage_changes/eco_type.rst
Outdated
Show resolved
Hide resolved
content/applications/inventory_and_mrp/plm/manage_changes/eco_type.rst
Outdated
Show resolved
Hide resolved
content/applications/inventory_and_mrp/plm/manage_changes/eco_type.rst
Outdated
Show resolved
Hide resolved
content/applications/inventory_and_mrp/plm/manage_changes/eco_type.rst
Outdated
Show resolved
Hide resolved
content/applications/inventory_and_mrp/plm/manage_changes/eco_type.rst
Outdated
Show resolved
Hide resolved
749ccae
to
32bbd77
Compare
32bbd77
to
8946af5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved (: @slinkous
Almost all my comments were optional suggestions, except for formatting the links and the one extra colon. This is such a good sign, since that means I only have nitpicks to "catch" in reviews, and it's only your 3rd PR! 😍
After you take a look, tag us-doc-review and @zac in the comments to ask for a final review :D
content/applications/inventory_and_mrp/plm/manage_changes/eco_type.rst
Outdated
Show resolved
Hide resolved
content/applications/inventory_and_mrp/plm/manage_changes/eco_type.rst
Outdated
Show resolved
Hide resolved
space Final changes, I hope
f8cbe4b
to
bb1db14
Compare
Ready for review! Tia @StraubCreative @odoo/us-doc-review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @slinkous
I think there's still more to go on this doc; it may need a significant rework to get it up to quality.
Overview of my comments:
- remove all vague/important-sounding sentences in favor of more concise language
- too much essentials content; I'd spend more time going into more depth around eco type configuration and use cases (PLM is an important app for mrp/engineer-based businesses, we should cover how it works much more deeply, in context for how it's used)
- the proposed rewrites in general seem superficial / not necessary? However, necessity is a core requirement for proposing changes...
More details below, please let me know if you have any questions!
Engineering change orders (ECOs) are categorized by the type of change they represent and their | ||
statuses are tracked via stages. Both |ECO| types and stages are defined by the user and can be | ||
customized to the specific needs of a business or industry. | ||
|
||
For example, an electronic chip manufacturer might use 'New Product Introduction', 'Product | ||
Improvement', 'Component Change', and 'Firmware Update' |ECO| types. Then, designers and engineers | ||
can focus on |ECOs| in the 'New Product Introduction' and 'Product Improvement' projects, avoiding | ||
unrelated supplier change or firmware update |ECOs|. | ||
ECO types | ||
========= | ||
|
||
Create ECO type | ||
=============== | ||
*Engineering change orders* (ECOs) require an |ECO| type to organize and track changes to products | ||
and bills of materials (BoMs). Like a :doc:`product | ||
type<../../inventory/product_management/configure/type>`, |ECO| types are highly customizable and | ||
specific to the needs of individual businesses. Each |ECO| type separates |ECOs| into different | ||
projects in the :guilabel:`PLM Overview`, ensuring collaborators and stakeholders only view and | ||
assist with relevant |BOM| improvements. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 12-24: can rewrite for clarity (can probably condense into 2 sentences)
=============== | ||
*Engineering change orders* (ECOs) require an |ECO| type to organize and track changes to products | ||
and bills of materials (BoMs). Like a :doc:`product | ||
type<../../inventory/product_management/configure/type>`, |ECO| types are highly customizable and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need a space here
type<../../inventory/product_management/configure/type>`, |ECO| types are highly customizable and | |
type <../../inventory/product_management/configure/type>`, |ECO| types are highly customizable and |
projects in the :guilabel:`PLM Overview`, ensuring collaborators and stakeholders only view and | ||
assist with relevant |BOM| improvements. | ||
|
||
.. example:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good example but could be simpler and more relevant to common use-cases (instead of changing defaults, could just add one new one to indicate that the business is scaling its engineering ops, more likely and easier to understand.)
Additionally, required approvers can be added to each stage, ensuring that changes to the production | ||
|BOM| cannot proceed until the approver reviews and approves the |ECO|. Doing so prevents errors on | ||
the production |BOM| by enforcing at least one review of suggested changes before they're applied on | ||
a production |BOM|. | ||
the production |BOM| by enforcing at least one review of suggested changes before they are applied | ||
on a production |BOM|. | ||
|
||
For best practice, there should be at least one *verification* stage, which is a stage with a | ||
The most common practice is to have at least one *verification* stage, which is a stage with a | ||
required approver, and one *closing* stage, which stores |ECOs| that have been either cancelled or | ||
approved for use as the next production |BOM|. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How
@@ -84,51 +109,40 @@ filling it in, click the :guilabel:`Add` button to finish adding the stage. | |||
Adding another stage helps the product manager track unassigned tasks. | |||
|
|||
.. image:: eco_type/create-stage.png | |||
:align: center | |||
:alt: Create a new stage in a project for an ECO type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lines 97-112: Essentials content, can omit.
Use case: configuring verification and closing stages | ||
----------------------------------------------------- | ||
|
||
To configure a verification stage, hover over the intended stage, and select the :guilabel:`⚙️ | ||
(gear)` icon. Then, click :guilabel:`Edit` to open a pop-up window. | ||
Verification stage | ||
~~~~~~~~~~~~~~~~~~ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disagree with the heading changes here
this isn't a use-case, it's a series of explicit instructions
|
||
Configure the verification stage in the edit stage pop-up window, by checking the box for | ||
To configure a verification stage, hover over the intended stage, and select the :icon:`fa-gear` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took too long for us to get here, can move up in context near line 84.
:align: center | ||
:alt: Show configurations of the closing stage. | ||
|
||
:alt: Configuration of the closing stage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 137+
Essentials content, can remove.
No description provided.