Skip to content
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

oca-gen-addon-readme: exits with no error if description.rst is not present #590

Open
aleuffre opened this issue Nov 21, 2023 · 17 comments
Open

Comments

@aleuffre
Copy link

Is your feature request related to a problem?
the oca-gen-addon-readme command (and of course its pre-commit hook) exits without error code if DESCRIPTION.rst is not present. I think this can lead to undesirable outcomes, like here in this module: OCA/sale-workflow#2771

where the description.rst was not present, the original committer added an empty README file to make the hooks happy and it was never updated even though there were other readme fragments in the folder.

Relevant method:

https://github.com/OCA/maintainer-tools/blob/master/tools/gen_addon_readme.py#L509-L526

Describe the solution you'd like
I think the pre-commit hook should fail with an error if no description.rst file exists. Maybe a command option that's then activated in OCA repos?

cc: @francesco-ooops

@aleuffre aleuffre changed the title oca-gen-addon-readme: fails silently if description.rst is not present oca-gen-addon-readme: exist with no error if description.rst is not present Nov 21, 2023
@francesco-ooops
Copy link

@aleuffre I think we can go directly for .md due to the ongoing transition :)

cc @OCA/oca-consultants

@aleuffre aleuffre changed the title oca-gen-addon-readme: exist with no error if description.rst is not present oca-gen-addon-readme: exits with no error if description.rst is not present Nov 29, 2023
@francesco-ooops
Copy link

On second analysis, I think also a check on presence of "USAGE" file in the module should be implemented.

If a module does not add anything to user interface per se, it can easily be stated in the file. If that's not the case, I think we can only benefit from having documentation explaining how to use the module.

It's easy to find many legacy modules describing what the module does in "DESCRIPTION", but not how to setup/employ their features, eg: https://github.com/OCA/partner-contact/tree/14.0/base_partner_sequence

@sbidoul @yajo what do you think?

@sbidoul
Copy link
Member

sbidoul commented Dec 3, 2023

When the readme generator was introduced, this was done to not override manually created README files and provide a smooth transition.

Today, I think we can now fail pre-commit if the main README (DESCRIPTION, USAGE, and maybe CONTEXT for new branches) fragments are not present.

@TumbaoJu
Copy link
Contributor

TumbaoJu commented Dec 5, 2023

I agree with the proposition done by @sbidoul.
The Read Me should have mandatory fragments such as : DESCRIPTION, CONTEXT AND USAGE.

@pedrobaeza
Copy link
Member

For me the only mandatory one should be DESCRIPTION.

@francesco-ooops
Copy link

For me the only mandatory one should be DESCRIPTION.

how would it hurt if the USAGE file would only contain a standard "This module has no impact on user interface" text?

It's useful for functionals and devs alike, and no cost for developers.

@pedrobaeza
Copy link
Member

It's an entry barrier that a module that doesn't have impact on that, requires to put such file for not having an error, and the text that you will find written by developers will not be ideal. Please don't make mandatory something that is not. If you want to add such text as functional, do it with the flows you are improving.

@francesco-ooops
Copy link

@pedrobaeza

It's an entry barrier that a module that doesn't have impact on that, requires to put such file for not having an error

I don't see what's the difference between having to add a DESCRIPTION and a USAGE file? if you can add one file, you can add two. All it takes is updating documentation to state both files are required and they will do it, just as with other requirements

and the text that you will find written by developers will not be ideal.

The same developers that can write DESCRIPTION can write USAGE. It doesn't take a special knowledge to write a few steps for USAGE, as nobody is requesting a full-fledged analysis of module use cases or a click-by-click-with-screenshots guide.

What it takes is to help module creator by improving instructions in USAGE template, in order to provide directions regarding what to put there (again, we'll publish a PR for templates asap). Keep in mind that up to this day, there is no mention of how to write documentation in OCA guidelines, and from a functional POV the effects are visible.

Also, I think it's a misconception that developers cannot write good documentation (tons of good USAGE files so far have been written, indeed, by devs). Still, what we're aiming for at the moment is at least decent, not good, documentation. As little as possible as to understand what to do when you open runboat.

If you want to add such text as functional, do it with the flows you are improving.

Yes, functionals hopefully will be always available to improve documentation, but that should not be considered as a cleanup service. Plus, if you're publishing a new module and need someone to do a functional test, you need at least to tell them how to use it.

I think it can only be positive if someone migrating a module can take a bit of time to write down a couple lines on how the final user would use this module.

On the contrary, you're not addressing the fact that legacy modules published when documentation was the last of the problems keep being migrated to this day without USAGE and sometimes even DESCRIPTION... This should fix that. Then of course we can always help improve documentation as soon as needed, but a USAGE file does not look to me like too much to ask.

@pedrobaeza
Copy link
Member

As said, for me the only mandatory file should be DESCRIPTION.

@francesco-ooops
Copy link

Fair enough, I just wanted to explain my reasoning :)

@TumbaoJu
Copy link
Contributor

TumbaoJu commented Dec 5, 2023

Thank you @francesco-ooops for the detailed explanation on why the USAGE fragment should be mandatory.

I totally agree with you and we have faced the same problems with OCA modules over the last years with modules we did not know how to use, so we just decided not to use them.

Sometimes, we could find a developer who would go look into the code and explain to us how to use it but sometimes, even the developer could not tell us how to use the module so, I really think USAGE should be mandatory.

For the CONTEXT fragment, it is new, so we can start to use it and see.

@rousseldenis
Copy link
Contributor

As said, for me the only mandatory file should be DESCRIPTION.

If we want to enhance the user experience with OCA modules, I think USAGE should be mandatory too. Even if its content is short. But it has another use than DESCRIPTION as this one is to describe the WHAT, USAGE is for the HOW.

@francesco-ooops
Copy link

Sometimes, we could find a developer who would go look into the code and explain to us how to use it but sometimes, even the developer could not tell us how to use the module so, I really think USAGE should be mandatory.

Very good point, it's a shared experience. It's frustrating to have to ask an internal dev to check the code and provide instructions on how to test an OCA module.

@yajo
Copy link
Member

yajo commented Dec 11, 2023

https://thegooddocsproject.dev/ exists to help people have good docs, which start by the readme (template and explanation here). If you want to restructure our readmes or their requirements, I think it's worth investigating.

@vdewulf
Copy link

vdewulf commented Aug 27, 2024

Hello,

We met today with the @OCA/oca-consultants group. We really think that making the "USAGE" mandatory could help improve the documentation of the OCA modules. We want a better discoverability of the OCA modules and a good documentation is a key to that objective. If developers struggle to make a good "usage" description, the consultants working group would be happy to give a hand there.

If we don't want to block a contribution if USAGE is left empty right now, we could still make it mandatory in our usage guidelines that are being written by the working group and will be soon published on the OCA website.
This could be a good starting point, in our opinion.

We'll be happy to discuss this matter during the next OCA Days edition in Liège, where a full day will be dedicated to the OCA Documentation Topic.

@yajo
Copy link
Member

yajo commented Aug 28, 2024

Let me just point out that there are modules that have no use because are just technical dependencies that need an implementation to actually expose some features.

However, in those cases the contributor can add a simple sentence explaining that, so it shouldn't be a big problem.

@francesco-ooops
Copy link

how would it hurt if the USAGE file would only contain a standard "This module has no impact on user interface" text?

It's useful for functionals and devs alike, and no cost for developers.

Exactly what I pointed here @yajo :)

Couldn't that be added to the template itself?

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

No branches or pull requests

8 participants