-
Notifications
You must be signed in to change notification settings - Fork 24
Renovate the docs #97
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
5782375 to
bfa886f
Compare
|
The stylesheet additions will be in another PR so that we can get the CNAME and docs structure changes mergedb. |
|
Imo CNAME should be put in its own file under the docs dir for easy visibility, just like what I did. If you change that I'd be happy to close my own pr (unless you already did) |
I'm not sure how it helps readability, especially since it just adds another file in the directory for a single line of text |
nezia1
left a comment
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.
Fantastic work. LGTM, added a review comment but non blocking
Not readability, visibility. The difference is, if anyone tries to change the domain in the future, for any reason, and runs into the issue that the CNAME gets changed each doc workflow run for no apparent reason, a file is way easier to find than a single line of text |
|
- option depth: 2 -> 3 - init readable docs - add readme.md symlink to docs/
92685b4 to
6bd78d3
Compare
6bd78d3 to
16f1519
Compare
|
Adding a couple more things. |
|
Addressed nezia's request and added a workflow to test build the docs. Please give feedback on the workflow, I'm pretty unfamiliar with them. |
nezia1
left a comment
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.
One last question, but LGTM otherwise. The section on defaultText is really good stuff :)
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.
Don't we build docs already in the deployment workflow? The workflow would fail anyways if it was not able to be built, so I'm unsure if we really need 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.
That occurs on merge. This workflow is a test workflow. So it's the difference between building the docs when a PR is opened to make sure it doesn't break anything, and building the docs after the PR is merged when it's either gonna break it or not.
|
|
The 29 errors on the link checker workflow is a bug with ndg atm, should be fine on our actual docs. |
- README no longer contains technical details on hjr - docs index now separated into Installation and Quirks - Environmental Variables section in Quirks
|
LGTM, but holding my review until ndg's bug is fixed |
I might just export the template and try to modify things myself. |
268e1fb to
59145c5
Compare
|
Alright, so the errors aren't a bug perse with ndg, rather, it's due to the javascript that adds anchors to headings not actually running at buildtime, even though it totally works fine on the published site. So, I've added anchors to the headings manually, and updated CONTRIBUTING to reflect that. What's left I want to do:
|
|
Superseded by #115. |
General PR to address some current issues of the docs. Namely:
Current Blockers:
Meta
Related Issue: #73
AI used to generate code included in this PR?: No
All Submissions: