-
Notifications
You must be signed in to change notification settings - Fork 236
chore: add doc format and install dependency #4741
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
Conversation
@colin-ho @ccmao1130 If you have time, please take a look at this PR. It's mainly about documentation - related content. Thanks. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4741 +/- ##
==========================================
+ Coverage 78.12% 78.27% +0.14%
==========================================
Files 873 877 +4
Lines 121840 123659 +1819
==========================================
+ Hits 95184 96788 +1604
- Misses 26656 26871 +215 🚀 New features to boost your workflow:
|
@ccmao1130 could you take a look at this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Jay-ju, thanks for opening this PR! The mkdocs.yml
and additional make
step and dependencies make sense. The only thing I want to push back on is deleting Docsearch because we wanted to integrate Algolia Docsearch instead of the native Mkdocs search for our documentation.
I can look into how we can improve our search crawler to reflect local searches
yamllint | ||
yamlfix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kevinzwang do these make sense to add?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have any yaml files in our docs afaik so let's not add these
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mkdocs.yml is yaml file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is true. If you'd like to lint that file though, could we add a linting step to the .pre-commit-config.yaml
instead? It looks like yamllint is a supported hook
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that the .pre-commit-config.yaml
file has some default configurations, and it can also be checked in precommit currently. I'm not sure if I need to define a separate YAML format file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's ok, we can worry about it some other time. In the mean time let's just not add these dependencies since they aren't explicitly used anywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- id: check-yaml There seems to be a check here, but it doesn't seem to be taking effect. However, I guess it's because the mkdocs.yml is relatively newly created and this check hasn't been implemented.
@ccmao1130 @kevinzwang Do you think an update is still needed? Please take a look when you have time. |
Changes Made
Goal 1: When making docs, directly include relevant dependencies to prevent errors during runtime.
Goal 2: Added formatting for mkdocs.yml.
Related Issues
Checklist
docs/mkdocs.yml
navigation