Skip to content
This repository was archived by the owner on Jun 13, 2025. It is now read-only.

Conversation

@Swatinem
Copy link
Contributor

@Swatinem Swatinem commented Feb 7, 2025

This creates stateful Matchers instead, to avoid re-compiling regex matchers in loops.


Builds on top of #1141, depends on codecov/shared#502

@Swatinem Swatinem requested a review from a team February 7, 2025 10:31
@Swatinem Swatinem self-assigned this Feb 7, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2025

This PR includes changes to shared. Please review them here: codecov/shared@7ba099f...e47e088

@Swatinem Swatinem marked this pull request as draft February 7, 2025 10:35
@ajay-sentry
Copy link
Contributor

ajay-sentry commented Feb 7, 2025

Is

if path and not match(component.paths, path):
# empty report since the path is not part of the component
return Report()
also a valid candidate to update?

@Swatinem
Copy link
Contributor Author

In theory, yes. But that code is not being run in a loop, so would not benefit from a stateful matcher.

@Swatinem Swatinem force-pushed the swatinem/opt-regex-matching branch from 3964169 to dd0f85d Compare February 12, 2025 09:12
@github-actions
Copy link
Contributor

github-actions bot commented Feb 12, 2025

This PR includes changes to shared. Please review them here: codecov/shared@016a756...3aea532

@ajay-sentry
Copy link
Contributor

@Swatinem Got it, I thought your shared PR removed the stateless match() fn but saw it was just at the bottom of the file, lgtm

Base automatically changed from swatinem/opt-paths to main February 13, 2025 09:44
This creates stateful `Matcher`s instead, to avoid re-compiling regex matchers in loops.
@Swatinem Swatinem force-pushed the swatinem/opt-regex-matching branch from dd0f85d to f8789d6 Compare February 13, 2025 10:30
@Swatinem Swatinem marked this pull request as ready for review February 13, 2025 10:30
@codecov
Copy link

codecov bot commented Feb 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.07%. Comparing base (115912a) to head (f8789d6).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1142   +/-   ##
=======================================
  Coverage   96.07%   96.07%           
=======================================
  Files         838      838           
  Lines       19773    19775    +2     
=======================================
+ Hits        18996    18998    +2     
  Misses        777      777           
Flag Coverage Δ
unit 95.96% <100.00%> (+<0.01%) ⬆️
unit-latest-uploader 95.96% <100.00%> (+<0.01%) ⬆️

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

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

@github-actions
Copy link
Contributor

✅ All tests successful. No failed tests were found.

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

@codecov-notifications
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@codecov-qa
Copy link

codecov-qa bot commented Feb 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.96%. Comparing base (115912a) to head (f8789d6).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

@Swatinem Swatinem added this pull request to the merge queue Feb 13, 2025
Merged via the queue into main with commit a12b039 Feb 13, 2025
18 of 19 checks passed
@Swatinem Swatinem deleted the swatinem/opt-regex-matching branch February 13, 2025 11:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants