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

docs: add opengraph metadata tags #860

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

docs: add opengraph metadata tags #860

wants to merge 15 commits into from

Conversation

shiftinv
Copy link
Member

@shiftinv shiftinv commented Nov 8, 2022

Summary

Adds sphinxext-opengraph to generate Open Graph metadata on all docs pages.

As an example, the resulting Discord embed looks like this:
image

Depends on wpilibsuite/sphinxext-opengraph#86 for now, but we're in no rush to add this anyway.

Checklist

  • If code changes were made, then they have been tested
    • I have updated the documentation to reflect the changes
    • I have formatted the code properly by running task lint
    • I have type-checked the code by running task pyright
  • This PR fixes an issue
  • This PR adds something new (e.g. new method or parameters)
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)

@shiftinv shiftinv added t: documentation Improvements or additions to documentation/examples t: enhancement New feature p: low Low priority s: blocked Issue/PR is blocked by other issues s: needs review Issue/PR is awaiting reviews labels Nov 8, 2022
@shiftinv shiftinv added this to the disnake v2.8 milestone Nov 8, 2022
…upstream feature

There's an upstream PR for this feature, but it has stalled for a while now.
I completely understand if the maintainers do not want to add such a feature,
however it's still useful in our particular case, so let's go with the slightly more convoluted way for now.
@shiftinv shiftinv marked this pull request as ready for review January 12, 2023 22:22
@shiftinv shiftinv removed the s: blocked Issue/PR is blocked by other issues label Jan 12, 2023
@shiftinv
Copy link
Member Author

Since the upstream PR this is waiting on has been stalled for a while now, I've decided to go with the slightly more fun complicated way of patching the event directly for now, adding on to the pile of sphinx hacks that are already there :p

If (and that's a big if, I'd understand if the maintainers don't want to add this feature) the other PR gets merged at some point, we can always update to that.

@shiftinv shiftinv removed this from the disnake v2.8 milestone Feb 5, 2023
@onerandomusername onerandomusername added the s: blocked Issue/PR is blocked by other issues label Feb 19, 2023
@onerandomusername
Copy link
Member

blocked by #722

@onerandomusername
Copy link
Member

would you please rebase on master and resolve conflicts?

@shiftinv shiftinv added s: in progress Issue/PR is being worked on and removed s: blocked Issue/PR is blocked by other issues labels Apr 9, 2023
@shiftinv
Copy link
Member Author

I've changed it to generate more concise descriptions, by only taking the first section into account.

The :ogp_disable: logic was also removed for now, as #722 changed things up quite a bit - should opengraph metadata be disabled for API reference pages again?
Since a common use-case is linking to specific items on those pages, and opengraph obviously doesn't account for that, those embeds aren't all that useful right now:
image

@onerandomusername
Copy link
Member

Perhaps we can launch it on all pages, and remove it from API reference pages if it turns out to not work too well. (We won't know unless we try it)

@shiftinv
Copy link
Member Author

Perhaps we can launch it on all pages, and remove it from API reference pages if it turns out to not work too well. (We won't know unless we try it)

Sound good to me, then.

@shiftinv shiftinv removed the s: in progress Issue/PR is being worked on label May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p: low Low priority s: needs review Issue/PR is awaiting reviews t: documentation Improvements or additions to documentation/examples t: enhancement New feature
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

2 participants