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

Pre-compile regular expressions before being used #7

Merged
merged 1 commit into from
May 6, 2017

Conversation

mattrobenolt
Copy link
Contributor

No description provided.

@mattrobenolt
Copy link
Contributor Author

bump

@ierror
Copy link

ierror commented Jul 3, 2014

any progress here?

@roycehaynes roycehaynes closed this May 5, 2017
@mattrobenolt
Copy link
Contributor Author

damn

@bryanhelmig
Copy link
Member

Yeah, this seemed like a decent patch. Any reason for the close instead of the merge?

Setting aside the fact that we haven't paid attention to this package for ... oh, 3 years ... 👀

@roycehaynes
Copy link
Contributor

@mattrobenolt - To be fair, this is a decent patch looking at the PR. Definitely worthy of reopen and merge....

@roycehaynes roycehaynes reopened this May 6, 2017
@roycehaynes roycehaynes merged commit 7aa8544 into zapier:master May 6, 2017
@roycehaynes
Copy link
Contributor

@mattrobenolt 6 tests failed after merge (should have ran locally before merging). Looking at this now, but may want to give your impl a second peep.

@roycehaynes
Copy link
Contributor

You can run tests by python from root dir python tests/test_email_reply_parser.py

@kageurufu
Copy link
Contributor

Python caches compiled regex regardless the first time they are compiled or used, unless this adds something on top of that I didn't see it as that important.

This will, however, potentially save a function call and dictionary lookup in practice

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.

5 participants