-
Notifications
You must be signed in to change notification settings - Fork 334
Replace QRegExp with QRegularExpression
#620
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
Conversation
|
Thank you for picking it up! What is your opinion about #606 (comment)? |
QRegExp with QRegularExpressionQRegExp with QRegularExpression
That looks good to me. I'll take a look at it. |
|
39f50ea to
c0c8b33
Compare
|
Force-pushed c0c8b33 addressing #606 (comment). |
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.
Approach ACK c0c8b33.
Suggesting to split the first commit into separated ones:
- factoring out the
extractFirstSuffixFromFilter()function (I'd prefer to useUpperCamelCasename convention for this new standalone function) - replacement
QRegExpwithQRegularExpression
Also please apply clang-format-diff.py for each your commit.
Extract the 'Extract first suffix from filter pattern...' functionality into a testable utility function
Co-authored-by: Pavol Rusnak <pavol@rusnak.io> Co-authored-by: Jarol Rodriguez <jarolrod@tutanota.com>
hebasto
left a comment
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.
ACK 67364eb
|
Code review and lightly tested ACK 67364eb |
Picking up #606 (labeled "Up for grabs") and applying #606 (review) and #606 (comment).
Replaces occurrences of
QRegExpusage withQRegularExpressionas part of the roadmap for Qt6 integration.Fixes #578