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

[FileCache#Find] N to 1 calls for file matching #14351

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

dafyddcrosby
Copy link
Contributor

@dafyddcrosby dafyddcrosby commented Apr 23, 2024

Description

This moves pre-computable values from the inner loop of #find to outside of the loop.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • I have run the pre-merge tests locally and they pass.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • If Gemfile.lock has changed, I have used --conservative to do it and included the full output in the Description above.
  • All new and existing tests passed.
  • All commits have been signed-off for the Developer Certificate of Origin.

@dafyddcrosby dafyddcrosby changed the title [FileCache#Find] Use Dir.glob with path instead of doing regexes [FileCache#Find] N to 1 calls for regex matching Apr 23, 2024
@dafyddcrosby dafyddcrosby force-pushed the filecache_find_dirglob branch 2 times, most recently from 4093aac to d65551e Compare April 23, 2024 22:34
@dafyddcrosby dafyddcrosby force-pushed the filecache_find_dirglob branch 2 times, most recently from b421935 to 9a2c309 Compare May 13, 2024 18:03
@dafyddcrosby dafyddcrosby changed the title [FileCache#Find] N to 1 calls for regex matching [FileCache#Find] N to 1 calls for file matching May 13, 2024
@dafyddcrosby dafyddcrosby marked this pull request as ready for review May 13, 2024 19:57
@dafyddcrosby dafyddcrosby requested review from a team as code owners May 13, 2024 19:57
@dafyddcrosby dafyddcrosby force-pushed the filecache_find_dirglob branch 3 times, most recently from 7297be6 to f48c75d Compare May 16, 2024 22:22
Copy link
Collaborator

@jaymzh jaymzh left a comment

Choose a reason for hiding this comment

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

I like this... but lets comment this method

@tpowell-progress tpowell-progress self-assigned this May 21, 2024
@tpowell-progress tpowell-progress added the Status: Waiting on Contributor A pull request that has unresolved requested actions from the author. label May 21, 2024
Instead of holding the reference to files the whole loop, use until and take
the head from the array

Signed-off-by: David Crosby <[email protected]>
The regular expression's capture is changed so that instead of capturing the
tail of the file string (ie cache path excluded), we capture the cache path
instead so that we can use delete_prefix calls to mutate the strings directly,
saving on allocations

Signed-off-by: David Crosby <[email protected]>
Copy link

sonarcloud bot commented May 21, 2024

Quality Gate Passed Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
38.7% Duplication on New Code

See analysis details on SonarCloud

@tpowell-progress tpowell-progress added Status: Waiting for Build Fix and removed Status: Waiting on Contributor A pull request that has unresolved requested actions from the author. labels May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants