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

docs: Drop wavedrom, reorder requirements #156

Merged
merged 1 commit into from
Jan 13, 2025
Merged

docs: Drop wavedrom, reorder requirements #156

merged 1 commit into from
Jan 13, 2025

Conversation

gastmaier
Copy link
Contributor

@gastmaier gastmaier commented Jan 13, 2025

PR Description

Sphinx-wavedrom [deps: cairocffi, CairoSVG] don't seem to be tested continuously, which leads to fatal bugs reaching production. The extension in particular has two red flags:

  • It renders on the client side, requiring wavedrom.js to be fetched from a different from a different domain, slowing down or even preventing the documentation if the domain is offline (cause of a previous problem).
  • Even though it renders on the client side in our user case, it still import cairo renderer during build (the root cause of the current issue)

Drop it in favor of just committing the wavedrom source (commented out in the page or as a text file in the same path) and the rendered svg.

Also, reorder requirements to have adi_doctools as the first requirement.

You can check the checkboxes below by inserting a 'x' between square brackets
(without any other characters or spaces) or just check them after publishing the PR.

If there is a breaking change, specify dependent PRs in description and
try to push all related PRs at the same time.

Without this dependency, the CI now takes 18s instead of 29s

PR Type

  • Bug fix (change that fixes an issue)
  • New feature (change that adds new functionality)
  • New test (change that adds new test program and/or testbench)
  • Breaking change (has dependencies in other repositories/testbenches)

PR Checklist

  • I have followed the code style guidelines
  • I have performed a self-review of changes
  • I have ran all testbenches affected by this PR
  • I have commented my code, at least hard-to-understand parts
  • I have signed off all commits from this PR
  • I have updated the documentation (wiki pages, ReadMe files, Copyright etc)
  • I have not introduced new Warnings/Errors on compilation/elaboration/simulation
  • I have set the verbosity level to none for the test program

Sphinx-wavedrom, cairocffi, CairoSVG don't seem to be tested
continuously, which leads to fatal bugs reaching production.
The extension in particular has two design oversights:
* It renders on the client side, requiring wavedrom.js to be
  fetched from a different from a different domain, slowing
  down or even preventing the documentation if the domain is
  offline (cause of a previous problem).
* Even though it renders on the client side in our user case, it
  still cairo renderer during build, even though it is unused (the
  root cause of the current issue)

Drop it in favor of just committing the wavedrom source (commented out
in the page or as a text file in the same path) and the rendered svg.

Also, reorder requirements to have adi_doctools as the first
requirement.

Signed-off-by: Jorge Marques <[email protected]>
@gastmaier gastmaier requested a review from StancaPop January 13, 2025 14:33
@IstvanZsSzekely IstvanZsSzekely added the documentation Improvements or additions to documentation label Jan 13, 2025
@gastmaier gastmaier merged commit 3009593 into main Jan 13, 2025
2 checks passed
@gastmaier gastmaier deleted the drop-wavedrom branch January 13, 2025 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants