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.

Sign up for GitHub

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

Jump to bottom

chore: collapse module spec and readme #13143

Merged
merged 20 commits into from
Sep 5, 2022
Merged

chore: collapse module spec and readme #13143

merged 20 commits into from
Sep 5, 2022

Conversation

alexanderbez
Copy link
Contributor

@alexanderbez alexanderbez commented Sep 2, 2022 •
edited by tac0turtle
Loading

Description

closes: #6779

  • Move spec into single readme
  • Perform some cleanup (e.g. params, etc...)

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@alexanderbez alexanderbez mentioned this pull request Sep 2, 2022
Closed
5 tasks
@alexanderbez alexanderbez marked this pull request as ready for review September 2, 2022 17:40
@alexanderbez alexanderbez requested a review from a team as a code owner September 2, 2022 17:40
julienrbrt
julienrbrt reviewed Sep 2, 2022
View reviewed changes
Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

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

Don't we do all modules in one PR? You need to modify this file too, so it points to the right README: https://raw.githubusercontent.com/cosmos/cosmos-sdk/ccdc355eea4eecac72395b7660b757ca45d394f1/x/README.md.
Which means you'll need to update https://github.com/cosmos/cosmos-sdk/blob/main/docs/pre.sh.

I think we should do all modules together: otherwise they won't appear anymore on docs.cosmos.network till pre.sh is updated.

@alexanderbez
Copy link
Contributor Author

Don't we do all modules in one PR?

No, that would be too much for one author.

I think we should do all modules together: otherwise they won't appear anymore on docs.cosmos.network till pre.sh is updated.

How? This is on main. I'd be shocked if docs.cosmos.network displays docs based off of main and not the latest release.

@julienrbrt
Copy link
Member

julienrbrt commented Sep 3, 2022 •
edited
Loading

Don't we do all modules in one PR?

No, that would be too much for one author.

I think we should do all modules together: otherwise they won't appear anymore on docs.cosmos.network till pre.sh is updated.

How? This is on main. I'd be shocked if docs.cosmos.network displays docs based off of main and not the latest

It will still appear in releases, but for those who bookmarked /main, the bank module spec will be broken.

@tac0turtle
Copy link
Member

lets do all in one, its not a heavy lift

@tac0turtle tac0turtle changed the title chore: collapse x/bank spec and readme chore: collapse module spec and readme Sep 4, 2022
@alexanderbez
Copy link
Contributor Author

We can do all in one, sure. But I still fail to see what this has to do with the webapp for displaying docs.

@alexanderbez
Copy link
Contributor Author

@marbar3778 can you comment on what modules you will take care of so I can do the rest, please?

@julienrbrt
Copy link
Member

julienrbrt commented Sep 5, 2022 •
edited
Loading

I picked up the rest. Now improving pre.sh so we can keep the same page split because:

previous:
image

current:
image

Btw x/genutil and x/simulation have no docs at all (as README I mean, but they have a doc.go, do we want to copy that content into a REAMDE?).

@julienrbrt
Copy link
Member

We can do all in one, sure. But I still fail to see what this has to do with the webapp for displaying docs.

This is because we build the docs based on the /spec of each module. So, the logic for building the docs needs to change if we remove the /spec folder.

@tac0turtle
Copy link
Member

I picked up the rest. Now improving pre.sh so we can keep the same page split because:

previous: image

current: image

Btw x/genutil and x/simulation have no docs at all (as README I mean, but they have a doc.go, do we want to copy that content into a REAMDE?).

this is the left menu, what about the right menu?

@julienrbrt
Copy link
Member

julienrbrt commented Sep 5, 2022 •
edited
Loading

this is the left menu, what about the right menu?

The right menu is awful because all headers show the same with one big file. So, you don't really see the page structure at all.
It would be great if our vuepress would behave like osmosis's. Then won't need that file splitting.

Fixed now :)

image

@tac0turtle tac0turtle enabled auto-merge (squash) September 5, 2022 12:24
@tac0turtle tac0turtle merged commit a0682b1 into main Sep 5, 2022
@tac0turtle tac0turtle deleted the bez/6779-bank-spec-refactor branch September 5, 2022 12:26
@julienrbrt julienrbrt mentioned this pull request Sep 5, 2022
Merged
19 tasks
@alexanderbez
Copy link
Contributor Author

Wow, you guys are amazing. Thank you for the hard work 👏

@julienrbrt julienrbrt mentioned this pull request Oct 5, 2022
Closed
Wryhder pushed a commit to Wryhder/cosmos-sdk that referenced this pull request Oct 26, 2022
@alexanderbez @tac0turtle
@julienrbrt @Wryhder
chore: collapse module spec and readme (cosmos#13143)
4fe84a2 
* updates

* add auth and separate vesting

* authz

* capability

* crisis

* epoching

* nft

* mint

* params

* upgrade

* fix website build

* finish other modules

* fix `pre.sh` and update modules title

* add docs

* display vesting in docs

* add page splitting for missing modules

* updates

* improve `pre.sh`

Co-authored-by: marbar3778 <marbar3778@yahoo.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tac0turtle tac0turtle tac0turtle approved these changes

@julienrbrt julienrbrt julienrbrt approved these changes

Assignees
No one assigned
Labels
C:x/auth C:x/authz C:x/bank C:x/capability C:x/crisis C:x/distribution distribution module related C:x/evidence C:x/feegrant C:x/gov C:x/group C:x/mint C:x/nft C:x/params C:x/slashing C:x/staking C:x/upgrade
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add readme.md to sub-folders
3 participants
@alexanderbez @julienrbrt @tac0turtle