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

[RFC] autogenerate INSTALL part of the README file most of the time #526

Open
legalsylvain opened this issue Mar 6, 2022 · 7 comments
Open

Comments

@legalsylvain
Copy link
Contributor

Hi all,

Rational

Most of the time, INSTALL section of the README.rst file is just a simple text to say which (external/python) library should be installed. The text could be autogenerated, parsing __manifest__.py (external_dependencies key) or (external_dependencies_override key in setup.py file). That way :

  • it avoids writing a text that can be guessed.
  • it avoid inconsistencies between the librairies mentionned in the code and the librairies mentionned in the INSTALL section, specially during code changing. (during migration, refactoring, etc...)

Proposal

Change the algorithm of oca_gen_readme

  • if no INSTALL.rst and no external_dependencies -> nothing. (-> No changes)
  • if no INSTALL.rst and external_dependencies is not null -> autogenerate text in the README.rst
  • if INSTALL.rst is present, use it. (-> No changes)

temporary minor inconvenience

  • when activating this feature, some README.rst file will be generated and pushed on pypi, if the text is not exactly the same.
    Note : 915 module versions over 11723 (8%) has external_dependencies defined. (but I guess that most of them has a INSTALL.rst file). (Ref)

CC main contributors : @sbidoul, @pedrobaeza, @guewen, @gurneyalex

thanks for your comments.

@pedrobaeza
Copy link
Member

Thanks for the suggestion. It's a good idea. Though, I have used INSTALL.rst in some occasions to mention that a field is computed and stored to warn about the installation time, so I would propose to always append to the content of the INSTALL.rst the auto-generated part (although having the possibility to double content for those existing ones).

@legalsylvain
Copy link
Contributor Author

thanks a lot for your comment @pedrobaeza.

the "append" is OK for me and it's better to maintain. It just means three thinks :

  • Once deployed, it will regenerate all the README.rst file and most of them will have duplicated information. (autogenerated, and coming from INSTALL.rst). So we will have to delete / update them manually. Not a big deal though, and repo maintainers can spend 5 minutes to do it on their repo.

  • Maybe the autogenerated text and the manual one will be complicated to concat. Here is for exemple a "complicated sample". (I think it the most complicated one)

  • We have to be sure that the autogenerated text is OK (in every case) and I'm not sure, regarding bin part. here is a proposal :

1 You need to install the following python libraries :

pip install bokeh==1.1.0
pip install py3o.formats
pip install xxx
...

2 You need to install the following system librairies :

sudo apt-get install default-jre
sudo apt-get install xxx
sudo apt-get install postgis
...

Other general question : some contributors includes in INSTALL.rst file, the list of the other OCA modules on which the module depends. Exemple. My PoV is that it's unecessary and I don't do it, when I contribute a module. Do you think it's interesting to autogenerate that text ?

@pedrobaeza
Copy link
Member

I don't think the concatenation will suppose any problem, as you only need to append the new paragraphs making sure 2 carrier line as provided for being detected as a new block.

About putting the module dependencies in the INSTALL, I do it when the module doesn't belong to the same repository, as newcomers (or not so new) don't know where to look for finding such dependencies. I know that with pip, such problem doesn't exist (it's the only advantage of this deploy system I appreciate), but we should do such tips. But generating it automatically is not possible, so let's keep it as manual.

@legalsylvain
Copy link
Contributor Author

I don't think the concatenation will suppose any problem, as you only need to append the new paragraphs making sure 2 carrier line as provided for being detected as a new block.

OK for me !

I do it when the module doesn't belong to the same repository, as newcomers (or not so new) don't know where to look for finding such dependencies.

Well, regarding that point, indeed, if the module is present in another repository (OCA has > 200 repositories), it's great to mention it.

[...] But generating it automatically is not possible, so let's keep it as manual.

What about to call pypi (https://warehouse.pypa.io/api-reference/) Let's imagine an OCA/web 12.0 module, dependending on product, base_technical_features and web_responsive. If we look at pypi :

-> https://pypi.org/project/odoo12-addon-product/ doesn't exists -> Odoo
-> https://pypi.org/project/odoo12-addon-base-technical-features/ exists -> OCA
-> https://pypi.org/project/odoo12-addon-web-responsive/ exists -> OCA

Then, scanning current repo, we know easily that web_responsive is in current repo.

I think it's possible to generate automatically a text like :

This module depends on other OCA modules:

Note : the url can be found in the website key of the package.

That could save time for developpers and avoid obsolete INSTALL.rst text, when dependencies are changing.

@pedrobaeza
Copy link
Member

If you think that can be feasible, it's OK. Not sure if we are avoiding the online connection in the generator or other point of failure.

@legalsylvain
Copy link
Contributor Author

Not sure if we are avoiding the online connection in the generator or other point of failure.

When executed locally, it could be a problem. If we don't have access to Internet or if pypi is down, it will prevent to generate readme files.
That is a choice. Don't have a clear point of view on that topic. Let's wait what other @OCA/tools-maintainers think.

When executed on the github CI or by the @OCA-git-bot, not a problem in my opinion as the process will fail if pypi is not available.

@gurneyalex
Copy link
Member

The one thing I struggle a lot with when installing OCA addons is not the external dependencies, it's the Odoo dependencies, and especially the other OCA dependencies from other repositories, because, in a context where pip is not used for OCA addons, getting all the required OCA repositories in the addons path can be tedious. So please make sure this is part of this feature.

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

3 participants