Skip to content

Conversation

Limsunoh
Copy link
Contributor

@Limsunoh Limsunoh commented Dec 4, 2024

I have made things!

Related issues

@sobolevn sobolevn closed this Dec 4, 2024
@sobolevn sobolevn reopened this Dec 4, 2024
@sobolevn
Copy link
Member

sobolevn commented Dec 4, 2024

(misclick)

@Limsunoh Limsunoh requested a review from sobolevn December 7, 2024 18:30
@intgr intgr self-assigned this Sep 19, 2025
Copy link
Contributor

@intgr intgr left a comment

Choose a reason for hiding this comment

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

Sorry, I'm a little apprehensive about this change.

I think I had a look at this some time ago and came to the conclusion that any actual subclass of BaseRenderer must assign strictly non-None value to the media_type field. If it's None, DRF would actually crash.

So None is actually not a valid value to these fields. It's just a placeholder in the "abstract base class".

Typehinting this as str | None might mislead someone who tries to implement their own Renderer.

🟢 But what we should do is add comments explaining this & move the allowlist_todo.txt entry into allowlist.txt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants