search

jazzband / django-silk

Silky smooth profiling for Django
MIT License
4.35k stars 333 forks source link

Remove unneeded content_type strip #642

Closed albertyw closed 1 year ago

albertyw commented 1 year ago

Based on the comment https://github.com/jazzband/django-silk/pull/641#pullrequestreview-1245196140, there's not a good reason to have these lines that attempt to strip content_type.

If strip() is a method on a string, strip() will return a string making the strip() a few lines below superfluous (s.strip().strip() == s.strip()). If strip() does trigger the try/catch, the try/catch will be ineffective anyways because the later strip() will also trigger an AttributeError. There is a possibility that a different type implements strip() in a way that multiple strips have different behavior, but it doesn't seem possible given how the _parse_content_type function is called.

codecov[bot] commented 1 year ago

Codecov Report

Merging #642 (3bd3de9) into master (602921e) will increase coverage by 0.06%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #642      +/-   ##
==========================================
+ Coverage   86.33%   86.40%   +0.06%     
==========================================
  Files          52       52              
  Lines        2093     2089       -4     
==========================================
- Hits         1807     1805       -2     
+ Misses        286      284       -2
Impacted Files Coverage Δ
silk/model_factory.py 84.41% <ø> (+0.58%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more