-
Notifications
You must be signed in to change notification settings - Fork 10k
[IMP] accouting/l10n_br: Adding NFC-e feature #12921
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
@vbe-odoo Here's the PR of NFC-e |
@samueljlieber Hello! I'm running the guideline tests on this last build, but it still saying that there's an error on a file called Tyro.srt that I don't know about. But my doc seems Ok to me. Let me know if I need to do any adjustments cc @vbe-odoo |
e6b2d53
to
064807c
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.
Hi @sclo-odoo! Thank you for your work on this PR!
I hope you don't mind, since there were a number of changes, I've made a commit (064807c) to address the following:
- Modified RST syntax to match conventions and guidelines
- Added additional context for instruction where it was needed
- Only added images that were necessary/helpful
With these changes, this PR should be all set to move to final review. However, @sclo-odoo please let me know your thoughts on my changes and I can make any final adjustments before sending to final review! Thank you! :)
@samueljlieber Hello! You can go forward with it. |
064807c
to
a510db2
Compare
Squashed commits in a510db2 |
Thanks for the update on the Brazilian page! Let me know what you prefer. Thank you |
Hey @afma-odoo! Im excited to see the updated guidelines! Im okay with you updaing this PR, but Im going to leave it up to @sclo-odoo to make the decision :) |
Hello @afma-odoo, thanks for the heads up! Moreover, we will use what you do here to have a real example besides the new guidelines. Thanks! |
Thank you @afma-odoo @vbe-odoo @samueljlieber |
Hi @samueljlieber @vbe-odoo @sclo-odoo! Here is the guideline/template that you can use for future updates. |
Thank you so much @afma-odoo ! Waiting for the changes get applied 💯 Let me know if we need to do anything more. |
a510db2
to
2e283ad
Compare
Adding the NFC-e configurations and workflows to the user guide.
2e283ad
to
3d27f89
Compare
Hi @samueljlieber @vbe-odoo @sclo-odoo! For your info, the "Localization overview" section includes all general info or content that applies to more than one app. Regarding images, we generally tend to use them sparingly and only when necessary to avoid the need for frequent updates with new versions. Additionally, wide images can be hard to read on lower-resolution screens. We also don't use markups like rectangles or arrows on screenshots. Could you check and remove anything unnecessary or update the images that need adjustments? Thank you! Please don’t hesitate to reach out if you have any questions. 🙂 Thanks for your patience; I truly appreciate it! |
@afma-odoo Hello! About the content, it seems very good and direct, to me! About the images, unfortunately those 2 ones are not being rendered, I don't know why haha This is already good to me, thank you so much for all changes. |
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.
These changes look great to me @afma-odoo, thank you for your work!
Could you check and remove anything unnecessary or update the images that need adjustments?
Would it be acceptable to address removing/updating the existing images in another PR? The images added in this PR do not include any markup, and seem to be helpful to contextualize the NFC-e feature.
I suggest that we merge this PR as is, and open another IMP to address the existing l10n_br doc images? WDYT?
Thank you again :)
@sclo-odoo @samueljlieber Great! I’m glad it works well for you
Sure, let's do that! I approved it and added the be-doc-review for the next step ;) Thank you for updating the page ;) |
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.
Thank you guys for the tremendous work on this!
@robodoo r+
Adding the NFC-e configurations and workflows to the user guide.
Suggestion is to add the separately from the other flows, Like the "Vendor Bill" flow, just because it's specificity
Documentation: https://docs.google.com/document/d/1s-OvUfxL6AZL37JSu_4eS-ruy4FV8rmHcaAgfvzm1LY/edit?usp=sharing
Screenshots are at: https://drive.google.com/drive/folders/1sZVYNTqyTUzioo9m6nbzgjhpAUQhUqgR?usp=sharing