-
Notifications
You must be signed in to change notification settings - Fork 10.7k
[ADD] accounting: add Hong Kong localisations #3657
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
|
Hi @tong-odoo we will review asap and keep you informed. |
7d761c9 to
0c43166
Compare
Donapi
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.
Hello Tong,
First of all, thank you for creating this documentation page, good job!
Here are some general comments that need to be taken into account when writing the doc:
- we write up to 100 characters per line (https://www.odoo.com/documentation/14.0/contributing/documentation/rst_guidelines.html?highlight=100%20characters#start-a-new-line-before-the-100th-character)
- add :alt: on images (https://www.odoo.com/documentation/14.0/contributing/documentation/content_guidelines.html?highlight=alt#alt-tags)
- compress your images using pngquant (https://www.odoo.com/documentation/14.0/contributing/documentation/content_guidelines.html?highlight=alt#alt-tags)
- rename image files (https://www.odoo.com/documentation/14.0/contributing/documentation/content_guidelines.html?highlight=alt#alt-tags)
- add links and references to internal documentation
- use guilabels when needed (Use the guilabel markup to identify any text of the interactive user interface (e.g., button labels, view titles, field names, lists items, …).
I sometimes made specific comments in my review but didn't comment each time an issue was recurrent.
If you are not used to write documentation and feel like it will take you too much time, I am happy to implement the changes and push the PR. Just let me know :)
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
0c43166 to
0d0351e
Compare
|
@Donapi Thanks for the review! I have updated the comments above. Please take a look. Since this QR code is a new feature, please feel free to let me know if you need the new Runbot to test out the features. |
0d0351e to
59dd59e
Compare
|
Hi @tong-odoo ! Let me know if you would need a review while @Donapi is off. |
|
@xpl-odoo Yes thanks. I updated the comments above. |
xpl-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.
Nice job @tong-odoo! A few comments to address and we are good to go ;)
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
content/applications/finance/fiscal_localizations/hong_kong.rst
Outdated
Show resolved
Hide resolved
59dd59e to
cdbf1d2
Compare
|
Hi @xpl-odoo, Thanks for your review! I updated all the issues based on your comments. Please take a look. |
AntoineVDV
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+
|
@tong-odoo @AntoineVDV linked pull request(s) odoo/odoo#113209 not ready. Linked PRs are not staged until all of them are ready. |
|
@AntoineVDV It seems this merged suddenly, however the old PR is not ready. Could I target master instead? |
|
@tong-odoo It merged because the blocking PR on odoo/odoo was closed. If this PR should not have been merged, you can create another PR with a revert commit, and continue the work later in another branch. |
Thanks, i will create a revert pr tonight |
|
@AntoineVDV I created a new revert PR #4304. It should target 16.0 only since only 16.0 is pushed the new commit. Please take a look. Thanks! |
tast-3042940