-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[ADD] accounting: incoterms #4572
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
3418379 to
98ad344
Compare
98ad344 to
f3ab3fc
Compare
dade-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.
Hi Tom!
Great PR - I made a few comments about the structure and the need to include instructions on how to create additional Incoterms.
Also, Loredana and I both agree that Reporting isn't the best section for Incoterms. Perhaps Customer invoices instead?
content/applications/finance/accounting/reporting/incoterms.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/accounting/reporting/incoterms.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/accounting/reporting/incoterms.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/accounting/reporting/incoterms.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/accounting/reporting/incoterms.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/accounting/reporting/incoterms.rst
Outdated
Show resolved
Hide resolved
f3ab3fc to
faccfec
Compare
|
@dade-odoo Thanks for your review! I addressed the few points you mentioned. As for the location of the doc, Jon put Intrastat here, and since he wanted to split from Incoterms, I figured it should be in the same place |
faccfec to
cc5ed78
Compare
dade-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.
Looks good to me @toaa-odoo :)
jcs-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.
Looks good
- change commit message and pr title:
|
UUUh sorry I pressed ctrl+enter by mistake, I just started reviewing :) |
jcs-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.
Hey @toaa-odoo and @dade-odoo
Looks good :)
Please change the commit message and pr title: [ADD] accounting: incoterms
Also, now that I read this intro, I don't think it belongs to the reporting section. I guess it is mostly useful for customer invoices to define the responsibility of the customer. wdyt?
Cheers, Jonathan
content/applications/finance/accounting/reporting/incoterms.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/accounting/reporting/incoterms.rst
Outdated
Show resolved
Hide resolved
cc5ed78 to
6259147
Compare
Seperating Incoterms section from "Intrastat" doc to make it a stand alone documentation. task-3305222
6259147 to
479567f
Compare
jcs-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.
@robodoo r+
Seperating Incoterms section from "Intrastat" doc to make it a stand alone documentation.
task-3305222