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

Use the local theme as a requirement #4

Merged
merged 7 commits into from Apr 2, 2021
Merged

Conversation

daniellimws
Copy link

@daniellimws daniellimws commented Mar 9, 2021

Try to fix the build error that occurs in #2

Edit: Great! The fix works!

mithro and others added 2 commits February 22, 2021 09:40
Signed-off-by: Tim 'mithro' Ansell <me@mith.ro>
Signed-off-by: Daniel Lim Wee Soong <weesoong.lim@gmail.com>
@daniellimws daniellimws force-pushed the master_doc branch 2 times, most recently from 8a0a2ae to 4ce9f80 Compare March 9, 2021 00:20
Signed-off-by: Daniel Lim Wee Soong <weesoong.lim@gmail.com>
@daniellimws
Copy link
Author

@mithro ping

@mithro
Copy link

mithro commented Mar 24, 2021

This doesn't seem to use the local theme as a requirement? I would expect the docs/requirements.txt would be adding the theme?

Signed-off-by: Daniel Lim Wee Soong <weesoong.lim@gmail.com>
Signed-off-by: Daniel Lim Wee Soong <weesoong.lim@gmail.com>
@daniellimws
Copy link
Author

daniellimws commented Mar 25, 2021

Oh right. I applied the changes in #3 to this branch (renaming sphinx_material to sphinx_symbiflow_theme

Signed-off-by: Daniel Lim Wee Soong <weesoong.lim@gmail.com>
Signed-off-by: Daniel Lim Wee Soong <weesoong.lim@gmail.com>
@daniellimws
Copy link
Author

daniellimws commented Mar 25, 2021

Apparently docs/requirements.txt doesn't need to contain the theme because readthedocs will automatically install our theme. I referred to the old theme and it also didn't need to have it in requirements.txt.

Checkout the preview. It's our theme now.

@mithro
Copy link

mithro commented Apr 2, 2021

I guess this LGTM, I wonder if we can reduce the change delta to upstream however?

@mithro mithro merged commit d7a8aa4 into f4pga:master Apr 2, 2021
@daniellimws
Copy link
Author

What do you mean?

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.

None yet

2 participants