Skip to content

Conversation

@knst
Copy link
Collaborator

@knst knst commented Feb 8, 2023

Issue being fixed or feature implemented

This refactoring helps to make code more specific and clear.
There's using syntax feature from modern C++ such as 'enum class', structure bindings in loops, declaration variables inside if/switch statements, etc.

What was done?

This PR is based on @PastaPastaPasta 's PR #4472

There excluded changes related to using std::optional. Let's decide firstly about Result class: #5109

How Has This Been Tested?

Run unit/functional tests

Breaking Changes

No breaking changes

Checklist:

  • I have performed a self-review of my own code
  • I have assigned this pull request to a milestone

@knst knst mentioned this pull request Feb 8, 2023
@knst knst added this to the 19 milestone Feb 8, 2023
@PastaPastaPasta
Copy link
Member

(Also, please address CI failures)

@knst knst marked this pull request as draft February 12, 2023 15:20
Co-authored-by: Pasta <pasta@dashboost.org
@knst
Copy link
Collaborator Author

knst commented Feb 13, 2023

commit 417612644014c26d2c679eaeabb5af6dba448643
    fix: return size_t without conversion to int

This changes are dropped, because confusion about size_t type in pushKV in univalue.h

Should be done after this backport:
Merge bitcoin#25611: univalue: Avoid brittle, narrowing and verbose integral type confusions

@knst knst marked this pull request as ready for review February 14, 2023 05:48
@knst knst requested a review from UdjinM6 February 14, 2023 05:48
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

one suggestion, LGTM otherwise

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK for squash merge

@PastaPastaPasta PastaPastaPasta merged commit 7681d63 into dashpay:develop Feb 19, 2023
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.

3 participants