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.

Sign up for GitHub

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

Jump to bottom

Complete docs #78

Merged
merged 18 commits into from
Sep 28, 2020
Merged

Complete docs #78

merged 18 commits into from
Sep 28, 2020

Conversation

mkurc-ant
Copy link
Contributor

Continuation of work of #71

@mkurc-ant mkurc-ant mentioned this pull request Sep 16, 2020
Closed
@mkurc-ant mkurc-ant force-pushed the complete-docs branch 8 times, most recently from 2be0527 to dcace8b Compare September 17, 2020 13:43
@mkurc-ant mkurc-ant changed the title [WIP] Complete docs Complete docs Sep 17, 2020
@mkurc-ant
Copy link
Contributor Author

@mithro I've created a build system that automatically converts V2X test cases to examples for documentation. It works like that:

When the documentation is built, a Python script scans the tests directory for README.rst files. When one is found then it copies the whole test subdirectory to docs/examples. Moreover it runs V2X on all Verilog files found there and stylizes generated XMLs for VPR using vtr-xml-utils. The script is aware of module hierarchy (it scans Verilog includes). This makes the final XMLs complete (no includes) and more compact than the ones that come out of V2X. Note that now golden XMLs are not used for documentation. The examples "toctree" section is also automatically generated by the script basing on discovered README.rst files.

For now I've disabled all currently present examples and added some new, simpler ones under tests/gates. Please tell me whether that documentation generation system is ok. If so I can start adding documentation for existing tests presumably in subsequent PRs.

@mkurc-ant mkurc-ant requested review from mithro and kgugala September 17, 2020 13:57
@mkurc-ant mkurc-ant force-pushed the complete-docs branch 2 times, most recently from 6d3c59f to fbf816b Compare September 23, 2020 15:52
@mkurc-ant
Copy link
Contributor Author

@mithro I've fixed conda package dependency issues and added back "VTR examples" documentation to the build system. Could you review?

mithro
mithro reviewed Sep 23, 2020
View reviewed changes
conf/environment.yml Outdated Show resolved Hide resolved
mithro
mithro reviewed Sep 23, 2020
View reviewed changes
docs/collect_examples.py Outdated Show resolved Hide resolved
mithro
mithro reviewed Sep 23, 2020
View reviewed changes
docs/requirements.txt Outdated Show resolved Hide resolved
mithro
mithro reviewed Sep 23, 2020
View reviewed changes
docs/requirements.txt Outdated Show resolved Hide resolved
mithro
mithro approved these changes Sep 23, 2020
View reviewed changes
Copy link
Collaborator

@mithro mithro left a comment

Choose a reason for hiding this comment

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

Just some minor comments, otherwise LGTM!

tests/dsp/dsp_in_registered/README.rst Outdated Show resolved Hide resolved
tests/vtr/full-adder/README.rst Show resolved Hide resolved
tests/vtr/lutff-pair/README.rst Show resolved Hide resolved
tests/vtr/lutff-pair/omux/omux.pb_type.xml Outdated Show resolved Hide resolved
@mithro
Copy link
Collaborator

mithro commented Sep 23, 2020

BTW - I would enable line numbers on the XML files too.

@mithro
Copy link
Collaborator

mithro commented Sep 23, 2020

Future addition -- when the test references something, add an include which highlights the referenced item.

@mithro
Copy link
Collaborator

mithro commented Sep 23, 2020 •
edited by unfurl-links bot
Loading

The rendered readthedocs output seems to be missing some of the examples? https://python-symbiflow-v2x.readthedocs.io/en/latest/examples.html ?

@mithro
Copy link
Collaborator

mithro commented Sep 23, 2020

Before

image

After

image

@mkurc-ant
Copy link
Contributor Author

@mithro I added back missing clock examples.

@mithro
Copy link
Collaborator

mithro commented Sep 24, 2020

@mkurc-ant - Using fixed verse floating versions is a complicated trade off. By using floating versions we will discover when dependencies break this code as soon as possible. The later we detect a problem, the more features have been added and code changed making it harder to know what caused the breakage.

If we don't use floating versions we do need to set up an "auto-roll" bot which pushes the version forward automatically. Generally with that approach we would have a floating requirements / environment.yml file and a "locked" version which is generated from the floatin versions with the values updated.

So I guess, if we are going to lock the version then you are committing to creating a system to automatically roll the versions.

@mithro
Copy link
Collaborator

mithro commented Sep 24, 2020

@mkurc-ant - The output is also missing the DSP example?

@mkurc-ant
Copy link
Contributor Author

@mithro In #71 DSP examples were missing as well, the "DSP" in TOC is just a placeholder. I'll add the DSP examples.

Regarding fixing package versions: My point is that I just want to avoid a situation when you suddenly discover that the doc building system is broken because some upstream package got updated. The documentation is built only when a change is made to V2X. So imagine that you have added a feature and built the docs. Then eg. a month (or more) later you want to add another feature and you discover that the docs build system does not work anymore because an upstream package got updated. You are then forced to fix the issue with the build system instead of focusing of the feature functionality. By freezing versions you ensure that the build system will work.

Anyways, I'd rather not focus now on creating an automatic version rolling system so I'll unlock all versions. Hopefully no breaking change has been pushed upstream already.

@mithro
Copy link
Collaborator

mithro commented Sep 27, 2020

@mkurc-ant Is this ready to merge? If so, can you please go through and resolve all my comments?

@mkurc-ant
Final documentation fixes
b48aa9e 
Signed-off-by: Maciej Kurc <[email protected]>
@mkurc-ant
Copy link
Contributor Author

@mithro I've fixed and resolved them all. Ready to merge.

@mithro
Copy link
Collaborator

mithro commented Sep 28, 2020

LGTM!

@mithro mithro merged commit 4b31933 into chipsalliance:master Sep 28, 2020
@mithro
Copy link
Collaborator

mithro commented Sep 28, 2020

@mkurc-ant - We should track down why symbolator is generating white backgrounds here. I thought we were using our custom version which should be generating transparent backgrounds by default?

@mithro mithro deleted the complete-docs branch September 28, 2020 14:07
@mithro mithro mentioned this pull request Sep 28, 2020
Open
@daniellimws
Copy link
Collaborator

@mithro I just realized hdl/symbolator#1 is not merged yet, that's why it is still generating white backgrounds.

@mithro
Copy link
Collaborator

mithro commented Oct 30, 2020

Should be merged now!

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

@mithro mithro mithro approved these changes

@kgugala kgugala Awaiting requested review from kgugala

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mkurc-ant @mithro @daniellimws