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] Migrate website to Docusaurus #3646

Merged
merged 1 commit into from Nov 23, 2023
Merged

[docs] Migrate website to Docusaurus #3646

merged 1 commit into from Nov 23, 2023

Conversation

jackkoenig
Copy link
Contributor

@jackkoenig jackkoenig commented Nov 22, 2023

There are several things that can be improved but I think should be done in follow on PRs:

  • Add search: https://docusaurus.io/docs/search#using-algolia-docsearch
  • Fix the order of pages in the explanations
  • Restore verilog-vs-chisel cookbook (see below)
  • Make API documentation page automatically update for new releases and include link to latest SNAPSHOT (API docs are hosted by s01.oss.sonatype.org)
  • Figure out what the landing page should be. It used to be the README, it is no longer, we can restore that behavior or enhance it

This removes cookbook verilog-vs-chisel which is unfortunate but it has
many issues with Docusaurus (via how it processes markdown with MDX).
We should resolve these issues and restore the page later.

Note that this page is broken on the website as is:
Screenshot 2023-11-22 at 11 13 03 AM

Contributor Checklist

  • Did you add Scaladoc to every public function/method?
  • Did you add at least one test demonstrating the PR?
  • Did you delete any extraneous printlns/debugging code?
  • Did you specify the type of improvement?
  • Did you add appropriate documentation in docs/src?
  • Did you request a desired merge strategy?
  • Did you add text to be included in the Release Notes for this change?

Type of Improvement

  • Documentation or website-related

Desired Merge Strategy

  • Squash

Release Notes

Introducing a complete facelift of the Chisel website, now built with Docusaurus 3.0.

Reviewer Checklist (only modified by reviewer)

  • Did you add the appropriate labels? (Select the most appropriate one based on the "Type of Improvement")
  • Did you mark the proper milestone (Bug fix: 3.5.x, 3.6.x, or 5.x depending on impact, API modification or big change: 6.0)?
  • Did you review?
  • Did you check whether all relevant Contributor checkboxes have been checked?
  • Did you do one of the following when ready to merge:
    • Squash: You/ the contributor Enable auto-merge (squash), clean up the commit message, and label with Please Merge.
    • Merge: Ensure that contributor has cleaned up their commit history, then merge with Create a merge commit.

@jackkoenig jackkoenig added DO NOT MERGE Documentation Only changing documentation labels Nov 22, 2023
@jackkoenig jackkoenig added this to the 6.0 milestone Nov 22, 2023
@jackkoenig jackkoenig force-pushed the docusaurus branch 5 times, most recently from b6c70e5 to a711612 Compare November 22, 2023 21:32
---

# Appendix

This section covers some less-common Chisel topics.

* [Differences between Chisel3 and Chisel2](chisel3-vs-chisel2)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I can tell, this document doesn't exist, it's a dead link on the current website.

Copy link
Member

@seldridge seldridge left a comment

Choose a reason for hiding this comment

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

LGTM

Questions:

  1. Do 14k lines of website/package-lock.json need to be committed?

website/Makefile Outdated
Comment on lines 27 to 32
$(this_dir)/docs/introduction.md: $(mdoc_out)/introduction.md
mkdir -p $(shell dirname $@)
cp -R $(mdoc_out)/* $(this_dir)/docs/
Copy link
Member

Choose a reason for hiding this comment

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

I've had issues with using mkdir -p in Makefile build rules. I usually write this as an order-only prerequisite on the directory. E.g.:

Suggested change
$(this_dir)/docs/introduction.md: $(mdoc_out)/introduction.md
mkdir -p $(shell dirname $@)
cp -R $(mdoc_out)/* $(this_dir)/docs/
$(this_dir)/docs/introduction.md: $(mdoc_out)/introduction.md | $(this_dir)/
cp -R $(mdoc_out)/* $(this_dir)/docs/
$(this_dir)/:
mkdir -p $@

You may be able to use a generic wildcard rule for the directory. I thought old macOS make has issues with this, though:

%/:
	mkdir -p $@

Ditto below.

website/Makefile Outdated Show resolved Hide resolved
website/Makefile Outdated Show resolved Hide resolved
website/docusaurus.config.js Show resolved Hide resolved
website/package.json Outdated Show resolved Hide resolved
website/sidebars.js Outdated Show resolved Hide resolved
@jackkoenig
Copy link
Contributor Author

Do 14k lines of website/package-lock.json need to be committed?

I'm not really sure, my Googling suggested that it is considered best practice, it makes it much easier to reproduce things exactly as they were, especially when "time traveling" back to an older revision. That being said, it also causes merge conflict pain and some people are against committing it so I'll go ahead and remove it. If, in the future, we decide it was a mistake to remove it, we can add it then.

This removes cookbook verilog-vs-chisel which is unfortunate but it has
many issues with Docusaurus (via how it processes markdown with MDX).
We should resolve these issues and restore the page later.
Copy link
Member

@seldridge seldridge left a comment

Choose a reason for hiding this comment

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

LGTM

@jackkoenig jackkoenig merged commit ff5b115 into main Nov 23, 2023
15 checks passed
@jackkoenig jackkoenig deleted the docusaurus branch November 23, 2023 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Only changing documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants