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

Add mediatypes #127

Merged
merged 9 commits into from
Jul 27, 2023
Merged

Add mediatypes #127

merged 9 commits into from
Jul 27, 2023

Conversation

hadro
Copy link
Collaborator

@hadro hadro commented Jul 24, 2023

No description provided.

@hadro hadro temporarily deployed to development July 24, 2023 21:01 — with GitHub Actions Inactive
@hadro hadro temporarily deployed to development July 25, 2023 18:09 — with GitHub Actions Inactive
@hadro hadro marked this pull request as ready for review July 25, 2023 18:19
@hadro hadro added this to the IIIF v3 Update - First steps milestone Jul 25, 2023
@hadro
Copy link
Collaborator Author

hadro commented Jul 25, 2023

@glenrobson @digitaldogsbody Ok, I've added a number of media types, and added some tests as well.
Would love a gut-check on this just to make sure it all looks OK by you.

Copy link
Collaborator

@digitaldogsbody digitaldogsbody 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 - my only thinking would be that it might be nice if the options in the choice block were from "most desirable" to "least desirable" (although as long as what we think should be the default is first so that clients that pick the first option by default get something usable, this is not hugely important), so maybe we could reverse the order of the new MP3 types so the higher bitrate ones are at the start of the list behind VBR. This is also very much something we can address later after the service is launched :)

@hadro
Copy link
Collaborator Author

hadro commented Jul 27, 2023

That makes solid sense to me, and an easy fix, I will make that change and push shortly!

@hadro hadro merged commit 367ebe9 into presentation-v3 Jul 27, 2023
3 checks passed
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