Skip to content

Conversation

@oco-odoo
Copy link
Contributor

@oco-odoo oco-odoo commented Oct 3, 2022

[toaa: documentation for the accounting engine report taskid: 3042880 ]

@oco-odoo oco-odoo requested a review from jcs-odoo October 3, 2022 16:06
@robodoo
Copy link
Collaborator

robodoo commented Oct 3, 2022

@C3POdoo C3POdoo requested a review from a team October 3, 2022 16:08
@oco-odoo
Copy link
Contributor Author

oco-odoo commented Oct 3, 2022

@jcs-odoo I still need to go over the helpers in the code, but let me know what you think of this one. I tried not to go too technical and keep explaining what was important for someone creating his own report. Hopefully I was clear enough ! ^^

@oco-odoo oco-odoo requested a review from tsb-odoo October 3, 2022 16:08
@oco-odoo oco-odoo force-pushed the 16.0-reportalypse-doc-oco branch from 3eafb40 to 5191640 Compare October 4, 2022 09:36
@C3POdoo C3POdoo requested a review from a team October 4, 2022 09:37
Copy link
Collaborator

@AntoineVDV AntoineVDV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only checked content/developer/howtos/accounting_localization.rst for which I was pinged but LGTM.

@jcs-odoo
Copy link
Contributor

jcs-odoo commented Oct 4, 2022

Awesome @oco-odoo :)
It requires a lot of work before we can merge it, though, but it's so great to have such content from you! It's a gold mine :)
Thanks for participating in the user doc. That means a lot!

@toaa-odoo
Copy link
Contributor

Accounting doc review was pinged by the bot and the PR is not in draft, but is it finished?

@jcs-odoo's comment seems to suggest otherwise. Let me know :)

@toaa-odoo toaa-odoo force-pushed the 16.0-reportalypse-doc-oco branch 11 times, most recently from ccbf71c to 7617e22 Compare October 25, 2022 11:19
@toaa-odoo toaa-odoo requested review from LoredanaLrpz and removed request for a team and jcs-odoo October 25, 2022 11:26
Copy link
Contributor

@LoredanaLrpz LoredanaLrpz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, hello, @toaa-odoo; I hope you're doing great; here's my review
Overall, I think you did great in making all these formulas comprehensible. (Plus, I couldn't find any broken link)

My main suggestions are

  • to give more explanations on what you describe is (or refer to a doc that explains it) in no more than two lines, but just to help readers understand
  • and to change the structure to make the creation steps clearer

I haven't read the last part in detail, so I'll check it afterward when the next force push is done :D

Wish you a great day!

  • The examples have a weird indentation (I suggested using PyCharm, you should be able to copy & paste - didn't change the content :))

image

@toaa-odoo toaa-odoo force-pushed the 16.0-reportalypse-doc-oco branch from 7617e22 to 275ab09 Compare November 4, 2022 14:39
@toaa-odoo
Copy link
Contributor

@LoredanaLrpz Don't re-review yet, I still have a few comments to address later :)

@toaa-odoo toaa-odoo force-pushed the 16.0-reportalypse-doc-oco branch from 275ab09 to 12105a4 Compare November 8, 2022 15:02
@toaa-odoo
Copy link
Contributor

@LoredanaLrpz Don't re-review yet, I still have a few comments to address later :)

Done! You can review :)

Copy link
Contributor

@xpl-odoo xpl-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job Tom, seems like you had fun with this one 🧠

Comment on lines 23 to 24
To access the accounting report creation interface, the :ref:`developer mode <developer-mode>`
has to be activated.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
To access the accounting report creation interface, the :ref:`developer mode <developer-mode>`
has to be activated.
Activate the :ref:`developer mode <developer-mode>` to access the accounting report creation interface

Suggestion
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@toaa-odoo you resolved this without changing or commenting.

Copy link
Contributor

@jcs-odoo jcs-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a quick comment here, but I wonder if the use of literal markups wouldn't be more appropriate than bold, when you write values and examples of formulas.
https://www.odoo.com/documentation/master/contributing/documentation/rst_cheat_sheet.html#technical-term-literal

@toaa-odoo toaa-odoo force-pushed the 16.0-reportalypse-doc-oco branch from f8bf397 to 469e743 Compare November 17, 2022 14:56
@toaa-odoo toaa-odoo force-pushed the 16.0-reportalypse-doc-oco branch 5 times, most recently from b4b56ba to 51c55bd Compare November 30, 2022 14:36
@oco-odoo oco-odoo force-pushed the 16.0-reportalypse-doc-oco branch from 51c55bd to a1bfa17 Compare December 1, 2022 14:26
@toaa-odoo toaa-odoo force-pushed the 16.0-reportalypse-doc-oco branch from a1bfa17 to 48001ea Compare December 2, 2022 07:05
@toaa-odoo toaa-odoo requested a review from xpl-odoo December 2, 2022 07:06
@C3POdoo C3POdoo requested a review from a team December 2, 2022 07:06
@xpl-odoo xpl-odoo removed their request for review December 5, 2022 11:30
Copy link
Contributor

@xpl-odoo xpl-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if/when you need another review from me.

@toaa-odoo
Copy link
Contributor

@oco-odoo You might want to have a look at @william-andre's comments

@toaa-odoo toaa-odoo force-pushed the 16.0-reportalypse-doc-oco branch 2 times, most recently from 5ed4e30 to a981458 Compare December 19, 2022 11:18
@toaa-odoo
Copy link
Contributor

@oco-odoo @william-andre @tsb-odoo @jcs-odoo Should you have any additional comments, let me know now. Otherwise, I'm considering this PR ready for merge @AntoineVDV.

Forward to master.

Copy link
Collaborator

@AntoineVDV AntoineVDV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robodoo delegate+
@robodoo delegate=toaa-odoo

Copy link
Contributor

@jcs-odoo jcs-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll push a few changes. (Tom, please check them with the "compare" button)

Comment on lines 13 to 14
Create a report
===============
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This heading is redundant with the main title, especially if it's the sole h2 heading of this doc.
It also puts all the content one level lower.
image
image

I'll remove it and put all other headings as h2

Comment on lines 23 to 24
To access the accounting report creation interface, the :ref:`developer mode <developer-mode>`
has to be activated.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@toaa-odoo you resolved this without changing or commenting.

Comment on lines 33 to 34
the page and the report is now available under :menuselection:`Accounting --> Reporting`. If a
report has no root report, it is considered to be a root report itself.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"if a report has..." is a different idea than the rest of the paragraph. I'll move it above.

Engine report for accounting.

taskid: 3042880
@jcs-odoo jcs-odoo force-pushed the 16.0-reportalypse-doc-oco branch from a981458 to 3f9e898 Compare December 21, 2022 15:21
@jcs-odoo
Copy link
Contributor

@robodoo r+

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants