-
Notifications
You must be signed in to change notification settings - Fork 11k
[IMP] mrp: assign lot/serial number to manuf products #9253
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
Conversation
|
This PR targets the un-managed branch odoo/documentation:17.0-mrp-bom-config-feku, it needs to be retargeted before it can be merged. |
|
Hello @ksc-odoo ! This PR is ready for your first round of peer review (: I have a note for you in the PR description -- appreciate your input in structuring the info! 😊 |
ksc-odoo
left a comment
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.
Great work, @Felicious -- everything seems good to me. And the current placement of this information seems fine to me -- but it also, in my opinion, does not seem too much for an admonition block, if you think it would be better placed elsewhere. Either way, this PR was very direct, too the point, and void of any suggestions (especially since it was so brief 😄 ). Once you consider my comments about the potential placement (which I hope are helpful), feel free to tag this for Tech Review whenever you're ready. Thanks!
...applications/inventory_and_mrp/manufacturing/basic_setup/configure_manufacturing_product.rst
Show resolved
Hide resolved
eeb4601 to
e162784
Compare
5bc251b to
5ac4733
Compare
|
Hi @samueljlieber ! This is a tiny update to the |
bb03a8d to
f2409d7
Compare
samueljlieber
left a comment
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 @Felicious, I think the information is presented in a good way, as is. It is clear to me that this step is an optional configuration 👍.
I have a suggestion about the images added in this PR– IMO they are not needed for this section, and including them breaks up the flow of the instruction too much. Its up to you to keep or remove them, but I think your written instructions are very clear and provide enough coverage :) WDYT?
I am approving and delegating merge to you, so that you can once #9239 is merged.
Thanks for your hard work!
..
@robodoo delegate=Felicious
...applications/inventory_and_mrp/manufacturing/basic_setup/configure_manufacturing_product.rst
Outdated
Show resolved
Hide resolved
|
I'm sorry. Branch |
|
p.s. per robot's comment please re-target the PR to 17.0 once ready before merge- tag me again so I can delegate :) |
5ac4733 to
1aaef9b
Compare
Co-authored-by: Sam Lieber (sali) <36018073+samueljlieber@users.noreply.github.com>
b165dc5 to
ea04187
Compare
|
Thanks @Felicious, looks good 👍 |
|
@robodoo r+ |
Summary of changes
reflinks to BoM doc with a relative path link using thedocdirectiveTask
Note for reviewers: does the structure of the info make sense? Do you think there's a better place to include this info? The lots/serial numbers config section is optional, but does it come off that way in the doc? If not, where in the manufacturing docs should I put this info? In my opinion, he section is too short to be its own doc, but too long to put in a tip admonition block. Let me know what you think!