Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Cleaner approach to HeaderMixin #285

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

kennethlove
Copy link
Member

satisfying

This PR introduces a backwards-compatible change to the HeaderMixin. Our existing method works but feels kind of heavy-handed. This approach is more in-line with Django's design, IMO, and doesn't feel as blunt.

Unfortunately, it only works if render_to_response is ultimately called, so I had to leave in the old approach as well. Maybe we should split it up into a new mixin?

This uses a new TemplateResponse which will let us more cleanly inject headers into the response.
@kennethlove kennethlove marked this pull request as draft November 18, 2021 21:51
@brack3t brack3t deleted a comment from codecov-commenter Nov 18, 2021
@codecov-commenter
Copy link

codecov-commenter commented Nov 18, 2021

Codecov Report

Merging #285 (2f1a4ec) into main (df847c9) will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #285      +/-   ##
==========================================
+ Coverage   97.72%   97.77%   +0.05%     
==========================================
  Files           8        8              
  Lines         484      495      +11     
  Branches       68       69       +1     
==========================================
+ Hits          473      484      +11     
  Misses          9        9              
  Partials        2        2              
Flag Coverage Δ
unittests 97.77% <100.00%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
braces/views/_other.py 100.00% <100.00%> (ø)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants