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

Don't raise W006 warning when app loader is specified. #1556

Merged

Conversation

tim-schilling
Copy link
Contributor

Fixes #1550

Thank you @leon-matthews for the report!

@codecov
Copy link

codecov bot commented Dec 18, 2021

Codecov Report

Merging #1556 (199eeb6) into main (66fe9c8) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1556      +/-   ##
==========================================
+ Coverage   86.62%   86.66%   +0.03%     
==========================================
  Files          36       36              
  Lines        1862     1867       +5     
  Branches      303      303              
==========================================
+ Hits         1613     1618       +5     
  Misses        175      175              
  Partials       74       74              
Impacted Files Coverage Δ
debug_toolbar/apps.py 93.65% <100.00%> (+0.54%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66fe9c8...199eeb6. Read the comment docs.

Copy link
Member

@matthiask matthiask left a comment

Choose a reason for hiding this comment

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

Nice! A good side-effect of this is that it annoys people even more who use the toolbar with DEBUG=False.

@tim-schilling
Copy link
Contributor Author

A good side-effect of this is that it annoys people even more who use the toolbar with DEBUG=False.

It does?

docs/changes.rst Outdated Show resolved Hide resolved
Co-authored-by: Matthias Kestenholz <mk@feinheit.ch>
@matthiask
Copy link
Member

A good side-effect of this is that it annoys people even more who use the toolbar with DEBUG=False.

It does?

Possibly, if they are using the old pattern of adding the cached Loader when DEBUG is falsy in the settings. Or not? Maybe I'm not thinking clearly (again).

@tim-schilling
Copy link
Contributor Author

tim-schilling commented Dec 18, 2021

From my understanding, if the user specifies any loader, those are the loaders that are used. No additional loaders, including the cached loader will be used.

If they don't specify loaders, then Django will specify the filesystem loader, and if app_dirs is True, then the app loader. Then if debug is false it will use the cached loader with whichever are used.

Sorry, my brain isn't working at 100%. I'm probably being pedantic.

Edit: relevant code: https://github.com/django/django/blob/main/django/template/engine.py#L20-L32

@tim-schilling tim-schilling merged commit c27261b into jazzband:main Dec 18, 2021
@tim-schilling tim-schilling deleted the fix-1550-template-options-check branch December 18, 2021 14:44
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.

False positive for debug_toolbar.W006?
2 participants