-
Notifications
You must be signed in to change notification settings - Fork 629
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
refactor(http): remove dead code and improve UserAgent
testing
#5120
Conversation
if (Array.isArray(str1)) { | ||
for (const el of str1) { | ||
if (lowerize(el) === lowerize(str2)) { | ||
return true; | ||
} | ||
} | ||
return false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
str1
is always string with all has
usages. This if-block is dead code.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5120 +/- ##
==========================================
+ Coverage 92.72% 92.87% +0.14%
==========================================
Files 479 479
Lines 38305 38291 -14
Branches 5394 5399 +5
==========================================
+ Hits 35520 35562 +42
+ Misses 2732 2676 -56
Partials 53 53 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few nits
http/user_agent.ts
Outdated
} | ||
return false; | ||
} | ||
function has(str1: string, str2: string): boolean { | ||
return lowerize(str2).indexOf(lowerize(str1)) !== -1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about .includes()
?
UserAgent
testing
This PR removes some dead code and improves typings in
http/user_agent.ts
, and also adds test cases which exercise uncovered lines.towards #3713