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

Switch to note admonition in readme #1216

Merged
merged 2 commits into from
Jan 18, 2024

Conversation

Gitznik
Copy link
Contributor

@Gitznik Gitznik commented Jan 17, 2024

  • I have added a news fragment under changelog.d/ (if the patch affects the end users)

Summary of changes

The current syntax does not render correctly and is not in line with the rest of the docs. I could not find any more usages of the other syntax in the docs.

See #1215 for the source of this PR.

Test plan

Tested by running nox -s build_docs and checking site/index.html

# command(s) to exercise these changes

@Gitznik Gitznik requested a review from chrysle January 17, 2024 09:49
@dukecat0
Copy link
Member

Seems like it only works on mkdocs, but not Github.

image

@Gitznik
Copy link
Contributor Author

Gitznik commented Jan 17, 2024

Seems like it only works on mkdocs, but not Github.

image

True. On github it's as broken as before. I won't have time to look into how to fix it for both before my vacation, so someone else please feel free to take this over.

@chrysle
Copy link
Contributor

chrysle commented Jan 17, 2024

Seems like it only works on mkdocs, but not Github.

Is that a problem? I always thought that's what a website is for.

@henryiii
Copy link
Contributor

Github does have a syntax for admonitions, and it doesn't match anything else. I wonder if it could be supported via a plugin, though?

@henryiii
Copy link
Contributor

And I'm pretty sure the reason it's currently broken is due to the square bracket part being on the same line. If you added an extra empty line (which IIRC is allowed), then formatters will keep it as is instead of squishing them together.

But that doesn't solve showing this in the docs. So far I've just used conditional blocks to get both, but something more elegant would be nice.

@henryiii
Copy link
Contributor

@henryiii
Copy link
Contributor

Screenshot 2024-01-18 at 10 57 51 AM

@dukecat0
Copy link
Member

@henryiii Thanks a lot for fixing this!

@dukecat0 dukecat0 enabled auto-merge (squash) January 18, 2024 16:06
@dukecat0 dukecat0 merged commit f52b8d4 into pypa:main Jan 18, 2024
14 checks passed
@Gitznik
Copy link
Contributor Author

Gitznik commented Jan 18, 2024

Great find @henryiii , thanks for taking this over 👌

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.

4 participants