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

Use Ocamlorg_static.Media.url for media #1163

Merged
merged 3 commits into from May 17, 2023
Merged

Use Ocamlorg_static.Media.url for media #1163

merged 3 commits into from May 17, 2023

Conversation

r17x
Copy link
Contributor

@r17x r17x commented May 7, 2023

Closes #1147


Hello @sabine , I have been make a PRs related with #1147 but not sure of my implementation is correct (meet the expectation).

Feel free for make a Review for this PRs

Thanks

@sabine
Copy link
Collaborator

sabine commented May 8, 2023

Looks good to me. We also need to go through all the templates to see where media files are being used and then use the new function to render the digest-URL. Media is everything that lives in the data/media folder.

ETA: every kind of data in the data/ folder has an associated module in Data, and some fields are paths of files in the data/media folder.

One type of media files that we won't be able to serve under digest URLs are the ones which are referenced from within the body of a markdown (.md) file because there's no way to render them.

@r17x r17x force-pushed the feat/media branch 2 times, most recently from 4febf8f to 5ac04b4 Compare May 8, 2023 17:09
@r17x
Copy link
Contributor Author

r17x commented May 13, 2023

Hello @sabine 👋

Sorry for my late updates.

I have been resolved conflicts after main branch updated and implemented Ocamlorg_static.Media.url in templates.

See the differences between this branch and ocaml.org (image attachments)

# in main branch or ocaml.org
-- /media/books/ocaml-scientific-computing.png
# in this branch after use Ocamlorg_static.Media.url
++ /media/_/OTg1ZWQ3NWMzZTZkYzVjNDRjN2VkY2FiNWJjNjEzMzk/books/ocaml-scientific-computing.png
image

Copy link
Collaborator

@sabine sabine left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for the contribution!

This issue is great to be resolved because, now, organizations can update their logo and there will be no issues with users' browsers still caching an old version. 👍

Never worry about taking time... we intentionally choose these issues so they are not time critical. After all, this is open source, and it's super nice of you to contribute! From our side, the goal is to provide a space where we can learn from each other and collaborate while making OCaml.org as useful to the community as we all can. 🙂

@r17x
Copy link
Contributor Author

r17x commented May 15, 2023

the goal is to provide a space where we can learn from each other and collaborate while making OCaml.org as useful to the community as we all can. 🙂
@sabine

The aspect of providing space for collaboration and learn from each other is an interesting in the OCaml community.

Thanks for providing the space.

@sabine sabine merged commit 8e7469f into ocaml:main May 17, 2023
3 checks passed
@r17x r17x deleted the feat/media branch May 17, 2023 19:46
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.

Serve data/media files under digest URLs
3 participants