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

Opengraph improvements #685

Closed

Conversation

anupam-tiwari
Copy link

@anupam-tiwari anupam-tiwari commented Jun 28, 2023

Describe your changes

Tasks

  • investigate how to modify the Python documentation so it shows JupySQL documentation
  • investigate how to display an image (most likely, we need to modify the html_meta)
  • check if it's possible to display one image by default (so we are not missing anything, we can later customize them)
  • create a sample image, you can use this template - note that this requires the draw.io app
  • add it to a sample article, let's start with duckdb's one

Issue number

Closes #253

Checklist before requesting a review


📚 Documentation preview 📚: https://jupysql--685.org.readthedocs.build/en/685/

@anupam-tiwari anupam-tiwari marked this pull request as ready for review June 28, 2023 17:03
@anupam-tiwari
Copy link
Author

@edublancas can you please check if this is the req!

Copy link

@edublancas edublancas left a comment

Choose a reason for hiding this comment

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

yep, this looks right!

the only missing item is:

  • check if it's possible to display one image by default (in case a given article doesn't have one)

@edublancas
Copy link

example:

given your changes, duckdb's section shows a preview image: https://www.opengraph.xyz/url/https%3A%2F%2Fjupysql--685.org.readthedocs.build%2Fen%2F685%2Fintegrations%2Fduckdb.html

but all other sections don't show anything: https://www.opengraph.xyz/url/https%3A%2F%2Fjupysql--685.org.readthedocs.build%2Fen%2F685%2Fintro.html

so we want to show a default image so something always shows up while we add custom ones per section

@neelasha23
Copy link

neelasha23 commented Jun 29, 2023

  1. Is there a way to test this before merging? (Point 1 can is verifiable but about the image? )
  2. Are we handling this : check if it's possible to display one image by default (so we are not missing anything, we can later customize them) : in a separate issue? Then this PR is not completely closing Issue opengraph improvements #253

doc/integrations/duckdb.md Outdated Show resolved Hide resolved
@anupam-tiwari
Copy link
Author

@edublancas I tried checking if image can be a static image but I couldn't find a way out, I think image link is the only way, please let me know if there is some other way!

@edublancas
Copy link

@anupam-tiwari check merge conflicts

@edublancas
Copy link

remember to explicitly request a review when this is ready

@anupam-tiwari
Copy link
Author

remember to explicitly request a review when this is ready

yes! this is not yet ready

@anupam-tiwari
Copy link
Author

add a new section to the developer guide

Added!

@edublancas
Copy link

hey @anupam-tiwari: I'm going to close this now, no need to keep working on it

@edublancas edublancas closed this Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

opengraph improvements
4 participants