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

Various improvements to demo-rollup/README.md #477

Merged
merged 7 commits into from
Jul 7, 2023

Conversation

neysofu
Copy link
Member

@neysofu neysofu commented Jul 7, 2023

Description

This is an attempt at making examples/demo-rollup/README.md more pleasing to the eye, simpler to follow, and overall better. Follow-up to #448.

Before: https://github.com/Sovereign-Labs/sovereign-sdk/blob/main/examples/demo-rollup/README.md
After: https://github.com/Sovereign-Labs/sovereign-sdk/blob/filippo/demo-rollup-improvs/examples/demo-rollup/README.md

Most notable changes:

  • Discord banner
  • Table of Contents generated with https://github.com/thlorenz/doctoc
  • I moved the Getting Started before the How to Customize This Example; I think it's reasonable to assume we want people to first try it out and not have new users look through the README to understand where the basic tutorial starts.
  • Widespread changes to formatting, occasional re-wording, fixed some typos. Also tried to make the use of second person more consistent throughout the tutorial but it's not perfect yet (I'll address it during the review based on feedback if we deem it as not good enough).

Linked Issues

This PR is not a solution to #476, which requires some deeper contents changes.

Testing

After modifying the README, I manually followed the process multiple time, also attempting slight variations and deviations. I believe it's working identical as before, as I haven't really changed the code blocks. Still, it would be great if someone else checked.

I recognize the Table of Contents can be a bit of a pain point, I wouldn't want anybody to forget to update it when modifying the README. We can either:

  • Check it's correct on CI (by running doctoc and diffing the file). 👈 EDIT: this is what I ended up doing.
  • Make it part of the stable release checklist.
  • Add a pre-commit hook.

I'm interested in feedback on these options.

@neysofu neysofu requested a review from cemozerr July 7, 2023 11:35
@codecov
Copy link

codecov bot commented Jul 7, 2023

Codecov Report

Merging #477 (3ce01f1) into main (369d639) will increase coverage by 0.0%.
The diff coverage is n/a.

see 2 files with indirect coverage changes

Copy link
Member

@cemozerr cemozerr left a comment

Choose a reason for hiding this comment

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

Looks great! Great formatting.

@neysofu neysofu force-pushed the filippo/demo-rollup-improvs branch from c19c9df to e273ab9 Compare July 7, 2023 14:21
@neysofu neysofu merged commit 4255415 into main Jul 7, 2023
@neysofu neysofu deleted the filippo/demo-rollup-improvs branch July 7, 2023 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants