-
Notifications
You must be signed in to change notification settings - Fork 22
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
Revamp of the documentation #1
Conversation
7a9d7d4
to
451b79b
Compare
9585019
to
3e397bc
Compare
2bbf4e7
to
06cd27c
Compare
sidebar_position: 1 | ||
--- | ||
|
||
# Architecture Decision Records (ADR) |
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.
this should stay in the sdk repo
@@ -0,0 +1,42 @@ | |||
--- | |||
sidebar_position: 0 |
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.
this should stay in the sdk
sidebar_position: 1 | ||
--- | ||
|
||
# Requests for Comments |
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.
ditto
@@ -0,0 +1,60 @@ | |||
# Specification of Modules | |||
|
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.
ditto with this folder
Agree with the comment Marko made above, and I would add the tooling documentation needs to stay in the SDK repo as well (so only some of the tooling). |
Thanks @julienrbrt ! I personally believe it should all be in one place, just had a chat to marko (so the above comment is ood) and I'm going to try the following. Im working on a script that copies over any MD files that have had changes and replaces the same existing MD files on this repo using a GitHub Actions workflow which is triggered on any deployment in the main sdk repo. |
I see, so it won't be removed from the SDK right? If so that makes sense as well. |
They wont be removed, those files will remain but also live in this repo |
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.
Can we have a PR with only the category and the docusaurus config and the action?
This is harder to review otherwise, my Google Chrome literally crashes viewing the diff.
This way there is as well a working base in the repo.
It would be nice as well if it can be configured simply without domain, so we can compare both website simultaneously (the current docs.cosmos.network and this one, at cosmos.github.io/cosmos-sdk-docs or another subdomain like docs-beta.cosmos.network)
.github/workflows/deploy.yml
Outdated
- run: yarn build | ||
|
||
- name: Deploy to GitHub Pages | ||
uses: peaceiris/actions-gh-pages@v3 |
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 should use the same action as in the SDK: https://github.com/cosmos/cosmos-sdk/blob/main/.github/workflows/deploy-docs.yml
There is no need to hardcode such thing in the action.
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.
Yes to all the above, ill make those changes !
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.
Can we have a PR with only the category and the docusaurus config and the action? This is harder to review otherwise, my Google Chrome literally crashes viewing the diff.
just to be clear you mean the category.json right?
.github/workflows/deploy.yml
Outdated
name: Deploy to GitHub Pages | ||
runs-on: ubuntu-latest | ||
env: | ||
CROWDIN_PERSONAL_TOKEN: ${{ secrets.CROWDIN_PERSONAL_TOKEN }} |
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.
What token is that?
.github/workflows/deploy.yml
Outdated
# https://github.com/actions/checkout/issues/13#issuecomment-724415212 | ||
# The GH actions bot is used by default if you didn't specify the two fields. | ||
# You can swap them out with your own user credentials. | ||
cname: docs.cosmos.network |
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.
This should be a file called CNAME at the root of the GH-pages branch
go.mod
Outdated
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.
This file should be deleted.
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.
tACK, pending a few changes (gh actions and unnecessary go.mod)
.github/workflows/deploy-docs.yml
Outdated
- name: Build 🔧 | ||
- run: yarn install --frozen-lockfile | ||
- run: yarn crowdin:sync | ||
- name: Clean Crowdin Front Matter issues |
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.
What is Crodwin and what is that clean.sh script?
It isn't present in the repo.
From what I can see it is for translations? Given that the website is in english, I do not think we 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.
Yeah its just something specific from docusaurus, ill remove
.github/workflows/deploy-docs.yml
Outdated
with: | ||
node-version: "16.x" | ||
|
||
# npm install npm should be removed when https://github.com/npm/cli/issues/4942 is fixed |
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.
# npm install npm should be removed when https://github.com/npm/cli/issues/4942 is fixed |
.github/workflows/deploy-docs.yml
Outdated
- main | ||
paths: | ||
- "docs/**" | ||
- "x/**/*.md" |
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.
- "x/**/*.md" |
There is no x folder here.
docusaurus.config.js
Outdated
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.
For compatibilities issues, we really need to keep the redirections working. Many website depends on it, and it is even in governance proposal.
This can definitely be done in a follow-up, but all redirections present in the docusaurus config in the SDK should as well be ported there.
Moreover, redirections should be set from all the links that have changed from the current docs version to this version.
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.
Ah i thought I mentioned in the description that the redirects section will be added to the versioning PR. So that piece of work will be added there. Thnx for checking up tho
* wip * Update glossary * review updates * update actions * remove path * Update deploy-docs.yml
closes part of: cosmos/cosmos-sdk#15705
This closes the main parts of the above issue including:
The following parts will happen in additional PRs
cosmos/cosmos-sdk
cosmos/cosmos-sdk
using github workflowsNote: This PR must be merged last.