Skip to content

Conversation

@w0xlt
Copy link
Contributor

@w0xlt w0xlt commented Jun 20, 2022

Picking up #606 (labeled "Up for grabs") and applying #606 (review) and #606 (comment).

Replaces occurrences of QRegExp usage with QRegularExpression as part of the roadmap for Qt6 integration.

Fixes #578

@hebasto
Copy link
Member

hebasto commented Jun 20, 2022

@w0xlt

Thank you for picking it up!

What is your opinion about #606 (comment)?

@hebasto hebasto changed the title qt: replace QRegExp with QRegularExpression Replace QRegExp with QRegularExpression Jun 20, 2022
@hebasto hebasto added Refactoring Qt 6 Transition to Qt 6 labels Jun 20, 2022
@w0xlt
Copy link
Contributor Author

w0xlt commented Jun 21, 2022

What is your opinion about #606 (comment)?

That looks good to me. I'll take a look at it.
Would you suggest a class/file to allocate the utility function?

@hebasto
Copy link
Member

hebasto commented Jun 21, 2022

What is your opinion about #606 (comment)?

That looks good to me. I'll take a look at it. Would you suggest a class/file to allocate the utility function?

namespace GUIUtil in qt/guiutil.{h,cpp}?

@w0xlt w0xlt force-pushed the regexp-obsolete branch 2 times, most recently from 39f50ea to c0c8b33 Compare June 21, 2022 15:39
@w0xlt
Copy link
Contributor Author

w0xlt commented Jun 21, 2022

Force-pushed c0c8b33 addressing #606 (comment).

Copy link
Member

@hebasto hebasto left a 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 use UpperCamelCase name convention for this new standalone function)
  • replacement QRegExp with QRegularExpression

Also please apply clang-format-diff.py for each your commit.

w0xlt and others added 3 commits June 21, 2022 19:16
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>
@w0xlt w0xlt force-pushed the regexp-obsolete branch from c0c8b33 to 67364eb Compare June 21, 2022 22:19
@w0xlt
Copy link
Contributor Author

w0xlt commented Jun 21, 2022

@hebasto thanks for the review.
Force-pushed 67364eb addressing your suggestions.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 67364eb

@laanwj
Copy link
Member

laanwj commented Jun 22, 2022

Code review and lightly tested ACK 67364eb

@laanwj laanwj merged commit 58b9d6c into bitcoin-core:master Jun 22, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 22, 2022
@w0xlt w0xlt deleted the regexp-obsolete branch June 23, 2022 11:12
@bitcoin-core bitcoin-core locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Qt 6 Transition to Qt 6 Refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace QRegExp with QRegularExpression

3 participants