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

doc: use BuildTheDocs, add CI workflow #2

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

umarcor
Copy link
Member

@umarcor umarcor commented Sep 24, 2021

This PR adds a CI workflow for building the docs and pushing to gh-pages automatically, using the BuildTheDocs Action and theme.

Copy link
Collaborator

@michael-etzkorn michael-etzkorn left a comment

Choose a reason for hiding this comment

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

One of the biggest issues with the documentation on this fork is references to pip install hdlparse since I believe this will pull Kevin's repo and version 1.0.4. We'll have to make our repo's package available on PyPI. We can also consider changing the name of modules to pyhdlparse to prevent installation issues.


> pip install setuptools

The easiest way to install pyHDLParser is from `PyPI <https://pypi.python.org/pypi/hdlparse>`_.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume one ticket of order is uploading a new package on PyPI. We should also consider changing the module names from hdlparse to pyhdlparse.

Copy link
Member Author

Choose a reason for hiding this comment

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

As commented below, I agree. However, we need to decide which namespace to use in PyPI, and modify the Symbolator dependencies accordingly.

doc/usage.rst Outdated Show resolved Hide resolved
You can access the Hdlparse Git repository from `Github
<https://github.com/kevinpt/hdlparse>`_. You can install direct from PyPI with the "pip"
command if you have it available.
You can access the Hdlparse Git repository from `Github <https://github.com/hdl/pyHDLParser>`_.
Copy link
Member

Choose a reason for hiding this comment

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

external URLs need 2x _ at the end.

Copy link
Member Author

@umarcor umarcor Sep 24, 2021

Choose a reason for hiding this comment

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

In fact, I did not review the rst syntax. I did:

  • Add .btd.yml, CI workflow and adapt conf.py for using the sphinx_btd_theme.
    • Remove sources for customising the previous theme (alabaster).
  • Split the index.rst into multiple .rst files and use a toctree to have the sidebar populated in the sphinx_btd_theme.
  • Add a genindex.rst placeholder and use it in the toctree.
  • Replace kevinpt/hdlparse with hdl/pyHDLParser and Hdlparse with pyHDLParser; but not the references to PyPI.
  • Break lines at max 120 chars.

The content of the README.rst is actually duplicated in the docs. We should probably remove most of the content from this file, and fix the syntax in getting.rst (see https://github.com/hdl/pyHDLParser/pull/2/files#diff-d6d6b4e691a98ef7d987710f8d3aeeed8eafecfbdfe0b990ece4d2a9bf81a347R16).

@umarcor
Copy link
Member Author

umarcor commented Sep 24, 2021

@michael-etzkorn you are correct about the naming, pip, etc. I did not change that on purpose. Let me explain:

  • Since we did not get any feedback from @kevinpt (yet), we might need to create new namespaces in PyPI for both hdlparse and symbolator, because I don't think we can take over his management.
    • I personally like the Symbolator name, so I would not want to change it, unless we are really forced to.
    • With regard to hdlparse, there are some other partially related projects which have some naming consistency (vhdl/pyVHDLModel, Paebbels/pyVHDLParser, edaa-org/pySystemVerilogModel, edaa-org/pyEDAA.ProjectModel, etc. (see edaa-org.github.io). It felt to me that hdl/hdlparse might be confusing, so I decided to rename this repo to hdl/pyHDLParser.
    • We might decide to release pyEDAA.Symbolator and pyEDAA.pyHDLParser in PyPI, so we can reuse the namespace in pip, even though we keep the repos in different orgs.
  • For now, I assume we will be manually cloning the repos and using PYTHONPATH or installing the repo. Therefore, I preserved hdlparse in the codebase to avoid breaking the integration between Symbolator and hdlparse|pyHDLParser. If we decide to rename the Python module from hdlparse to pyVHDLParser (which I do agree with), that needs to be done through two sibling PRs: one here and one in hdl/symbolator.

michael-etzkorn pushed a commit that referenced this pull request Oct 2, 2021
@Paebbels
Copy link
Member

@umarcor converted this to a draft as it's not yet ready for review, right?

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.

3 participants