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

Implement a quick find regex for license matching #225

Merged
merged 1 commit into from
Jan 5, 2024
Merged

Conversation

goneall
Copy link
Member

@goneall goneall commented Jan 3, 2024

Potential fix for #165

It seems to speed things up - not quite fast enough to get rid of the "slow tests" flags, but a noticeable improvement.

@pmonks - review / give it a try and let me know how it does. This implements the algorithm we were discussing.

Potential fix for #165

Signed-off-by: Gary O'Neall <gary@sourceauditor.com>
@pmonks
Copy link
Collaborator

pmonks commented Jan 5, 2024

Reviewing now. Thanks for turning this around so fast @goneall!

@pmonks
Copy link
Collaborator

pmonks commented Jan 5, 2024

Ok so running my default (smaller) test suite here took almost 10 minutes with v1.1.10, and only 35 seconds with the issue165 branch. I also ran it with my full (larger) test suite (which normally takes around an hour with v1.1.10), and with the issue165 branch it took just over 2 minutes. All tests were run with pre-downloaded & cached v3.22 SPDX license list and associated assets (so the time to load those assets was negligible - less than a second each time).

All of my tests passed in each case, so functionally it appears to be working well too.

tl;dr: this is a MASSIVE performance improvement!! 🎉

@goneall goneall marked this pull request as ready for review January 5, 2024 23:06
@goneall
Copy link
Member Author

goneall commented Jan 5, 2024

@pmonks - Thanks for the review. Wow! More of an improvement than I thought. Thanks for the design suggestions.

I'll go ahead and merge it in.

@goneall goneall merged commit e7212a6 into master Jan 5, 2024
1 check passed
@goneall goneall deleted the issue165 branch January 5, 2024 23:08
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.

2 participants