Skip to content

Conversation

3imed-jaberi
Copy link
Member

Checklist

  • I have ensured my pull request is not behind the main or master branch of the original repository.
  • I have rebased all commits where necessary so that reviewing this pull request can be done without having to merge it first.
  • I have written a commit message that passes commitlint linting.
  • I have ensured that my code changes pass linting tests.
  • I have ensured that my code changes pass unit tests.
  • I have described my pull request and the reasons for code changes along with context if necessary.

Copy link

codecov bot commented Jun 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.90%. Comparing base (2f6e814) to head (40d41e6).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1886   +/-   ##
=======================================
  Coverage   99.90%   99.90%           
=======================================
  Files           9        9           
  Lines        2046     2048    +2     
=======================================
+ Hits         2044     2046    +2     
  Misses          2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@3imed-jaberi
Copy link
Member Author

@jonathanong I’ve removed cache-content-type because it’s not in sync with the latest mime-types. I think we should rely directly on the upstream mime-types package instead. If you’re on board, we can move forward with this PR. Otherwise, happy to close it!

@3imed-jaberi 3imed-jaberi requested a review from jonathanong June 5, 2025 13:31
@jonathanong
Copy link
Member

@fengmk2 @dead-horse why do we need to cache the content type? I think all the mime types are already in memory

@3imed-jaberi 3imed-jaberi force-pushed the replace-cache-content-type-with-mime-type-direct branch from 2d8d15c to c32e378 Compare June 5, 2025 21:35
@jonathanong
Copy link
Member

@3imed-jaberi my preference would be to just remove caching entirely

@3imed-jaberi 3imed-jaberi changed the title feat: replcae cache-content-type with mime-types directly (keeps ylru cache) feat: replace cache-content-type with mime-types directly Jun 5, 2025
@3imed-jaberi
Copy link
Member Author

@jonathanong done 🚀 !

@fengmk2
Copy link
Member

fengmk2 commented Jun 6, 2025

Cache is no longer needed +1

@3imed-jaberi 3imed-jaberi merged commit 0acad8f into master Jun 6, 2025
6 checks passed
@3imed-jaberi 3imed-jaberi deleted the replace-cache-content-type-with-mime-type-direct branch June 6, 2025 12:11
@chunlampang chunlampang mentioned this pull request Jul 31, 2025
3 tasks
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.

3 participants