-
Notifications
You must be signed in to change notification settings - Fork 10.1k
[IMP] accounting/l10n_co: additional context #5936
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
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 @samueljlieber - your updates look good. I made a few comments/edits to the doc in general. Feel free to reach out to discuss any of them if you want :)
@@ -142,48 +166,56 @@ The partner's responsibility codes (section 53 in the RUT document) are included | |||
electronic invoicing module, as it is required by the |DIAN|. |
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.
Is "RUT document" something that all users familiar with Colombian accounting will understand? Otherwise it would be good to clarify what this document is or at least what the abbreviation stands for.
1edc245
to
48ba2f8
Compare
Hi @dade-odoo thank you for your review and suggestions for improvement! I implemented them in 48ba2f8. |
Hey @samueljlieber - I'll go ahead and approve this and pass it on to @odoo/be-doc-review, but I am still seeing a typo on line 89 - "modules are installed" or "module's installed" seem more appropriate to me :) I'll let you handle it |
48ba2f8
to
b433243
Compare
Hi @dade-odoo you are right, thank you for catching that! I just pushed a commit changing line 89 to "modules are installed" in b433243 |
Hi @odoo/be-doc-review this PR is ready for your review 🙂 |
b433243
to
8433fdf
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.
Not a lot of changes, but maybe that's because this was WAY shorter!
Co-authored-by: dade-odoo <87431108+dade-odoo@users.noreply.github.com> Co-authored-by: larm-odoo <121518652+larm-odoo@users.noreply.github.com>
02e2bc5
to
a359dfe
Compare
Thank you @larm-odoo for your review! I implemented most of your suggestions in a359dfe. @StraubCreative this PR should be good to go. 👍 |
@robodoo r+ |
closes #5936 Signed-off-by: Zachary Straub (zst) <zst@odoo.com> Co-authored-by: dade-odoo <87431108+dade-odoo@users.noreply.github.com> Co-authored-by: larm-odoo <121518652+larm-odoo@users.noreply.github.com>
@samueljlieber @StraubCreative this pull request has forward-port PRs awaiting action (not merged or closed): |
1 similar comment
@samueljlieber @StraubCreative this pull request has forward-port PRs awaiting action (not merged or closed): |
@samueljlieber @StraubCreative this pull request has forward-port PRs awaiting action (not merged or closed): |
This PR improves the content of the Colombian localization documentation by adding additional context to most sections per #4425.
This PR can be FWP with small edits needed for 15.0 and 16.0.