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

README.md:better markdown syntax for lists #165

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RandyMcMillan
Copy link

No description provided.

@RandyMcMillan
Copy link
Author

This doesnt effect how README.md is rendered on GitHub.com.
It enables correct list rendering in other markdown clients.

@RandyMcMillan
Copy link
Author

Before:

pr-165-before

After:

pr-165-after

Copy link
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

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

Good catch. When looking at it though I see a few more things that could be improved. Do you mind fixing these as well?

  • The first 3 bullets under "This crate can be used for the..." can all be at the same level (the last 2 are not subpoints of the first one).
  • The other list (Database Options, Blockchain Options, etc.) do not need indentation at the first level, and the second level can get 4 spaces of indentation.

Roughly I'd like us to try to adhere to the Google Markdown Styleguide, so that markdown is rendered the same everywhere. Thanks for catching this!

@RandyMcMillan RandyMcMillan force-pushed the 1700676420/fdb9d18/6f61d2e-readme-lists branch from d425df2 to 6d84b96 Compare November 28, 2023 23:03
@RandyMcMillan
Copy link
Author

RandyMcMillan commented Nov 28, 2023

6d84b96: update per suggestions

view rendered README.md: https://github.com/bitcoindevkit/bdk-cli/blob/6d84b9601a060e7765aaf819ad30a5201de14195/README.md

6d84b96

feel free to cherry-pick 6d84b96 and close if 854f072 isn't desirable.

@thunderbiscuit
Copy link
Member

thunderbiscuit commented Jan 12, 2024

@RandyMcMillan if you can give me commit access to your PR I'll be able to push some small fixes and get this merged. I'm trying to clean up some of the PRs and move stuff along, sorry it's slow going.

It's apparently a small checkbox that says "Allow edits by maintainers".

@RandyMcMillan
Copy link
Author

RandyMcMillan commented Jan 14, 2024

if there is something else I need to do - please tell me.

Screen Shot 2024-01-14 at 10 34 38 AM

@thunderbiscuit
Copy link
Member

Actually this will (I think) work, but in general you don't need to grant complete access to your repo for maintainers to be able to push commits on a PR.

I think this is the article to take a look at: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

@thunderbiscuit thunderbiscuit force-pushed the 1700676420/fdb9d18/6f61d2e-readme-lists branch from ec720c9 to 7c2dc52 Compare January 14, 2024 16:16
Copy link
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

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

ACK 7c2dc52. Thanks for the contribution!

@thunderbiscuit
Copy link
Member

@notmandatory I don't have permissions to merge this because of the protections on the master branch. The PR currently fails the CI but this is a docs PR and I don't want to get into fixing all the pins on the dependencies here.

@notmandatory
Copy link
Member

I think we need to do a couple things before we can merge this PR:

  1. publish release bdk 0.29.1 which has MSRV fixes or just bump MSRV to 1.63 and fix CI in a 0.30.0 version
  2. update bdk-cli to use bdk 0.29.1 and fix any other msrv issues

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