Skip to content

gh-84852: Add basic font MIME types #113592

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

gh-84852: Add basic font MIME types #113592

wants to merge 1 commit into from

Conversation

MasterInQuestion
Copy link

@MasterInQuestion MasterInQuestion commented Dec 31, 2023 •
edited by bedevere-app bot
Loading

    Namely:
    |*| ".ttf": "font/ttf"
    |*| ".otf", ".otc", ".ttc": "font/otf"
    |*| ".woff": "font/woff"
    |*| ".woff2": "font/woff2"

    References:
    https://developer.mozilla.org/en-US/docs/Web/CSS/@font-face#font_mime_types
    https://developer.mozilla.org/en-US/docs/Web/CSS/@font-face/src#font_formats
    https://en.wikipedia.org/wiki/OpenType
    https://en.wikipedia.org/wiki/TrueType

@MasterInQuestion MasterInQuestion requested a review from a team as a code owner December 31, 2023 00:33
@ghost
Copy link

ghost commented Dec 31, 2023 •
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Dec 31, 2023

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

serhiy-storchaka
serhiy-storchaka requested changes Jan 26, 2024
View reviewed changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

I think that it is a good idea. But this change needs a NEWS file and tests. Please open an issue for reference.

@bedevere-app
Copy link

bedevere-app bot commented Jan 26, 2024

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@MasterInQuestion
Copy link
Author

MasterInQuestion commented Mar 25, 2024 •
edited
Loading

    How shall I write the NEWS file..?
    Linking directly to [ https://github.com/python/cpython/pull/113592/commits/5e42988fc5ae8dadf2d24ff4bb108a2876264b5d ]?

    The tests... It won't break anything, semantically verified.

@serhiy-storchaka
Copy link
Member

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

As for the tests, you should add cases related to your changes in Lib/test/test_mimetypes.py. No need to add cases for all changes, but I think that it would be interesting to add checks for guess_all_extensions('font/otf') and guess_extension('font/otf'). Make your changes consistent with other similar checks.

@serhiy-storchaka
Copy link
Member

There is already open issue for adding fonts to mimetypes: #84852.

@serhiy-storchaka serhiy-storchaka changed the title Add basic font MIME types gh-84852: Add basic font MIME types Mar 25, 2024
@bedevere-app bedevere-app bot mentioned this pull request Mar 25, 2024
Closed
@MasterInQuestion
Copy link
Author

MasterInQuestion commented Mar 25, 2024 •
edited
Loading

    Finding issue for no issue...

    I did read.
    But the message... would:
    Add basic font MIME types: https://github.com/python/cpython/pull/113592/commits/5e42988fc5ae8dadf2d24ff4bb108a2876264b5d
    ; suffice?

    I believe the "test" files don't affect the eventual execution?
    If so... why bother?

@serhiy-storchaka
Copy link
Member

The link is not needed, but add the list of added MIME types or file extensions.

Tests are needed to check that the change makes sense and that the following changes will not unintentionally cancel them. For example, there is an alternate patch for #84852, conflicting with this PR.

@MasterInQuestion
Copy link
Author

    Is not the whole change most clear simply tracing the link..?

    Rather than spending time on tests that might not work, or might be outright malapropos...
    Wouldn't it be more appropriate, to focus on the actual coding itself?

@serhiy-storchaka
Copy link
Member

That's not how it works in software development. "Coding" without tests is meaningless.

@MasterInQuestion
Copy link
Author

    So shall be testing without "Coding".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@serhiy-storchaka serhiy-storchaka Awaiting requested review from serhiy-storchaka

Assignees
No one assigned
Labels
awaiting changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MasterInQuestion @serhiy-storchaka