Skip to content

Conversation

achow101
Copy link
Member

In a couple of places in the wallet, errors are std::string. In order for these errors to be translated, change them to use bilingual_str.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jun 24, 2021

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #19602 (wallet: Migrate legacy wallets to descriptor wallets by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

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 b1eeae2.

While touching this code, could string duplication be eliminated in wallet/scriptpubkeyman.cpp?

@achow101 achow101 force-pushed the wallet-bilingualstr branch from b1eeae2 to ffc8ff5 Compare June 28, 2021 21:46
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 ffc8ff5

achow101 added 3 commits July 1, 2021 12:57
For GetNewDestination, GetNewChangeDestination, and
GetReservedDestination, use bilingual_str for
errors
@achow101 achow101 force-pushed the wallet-bilingualstr branch from ffc8ff5 to 92993aa Compare July 1, 2021 17:03
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.

re-ACK 92993aa, only rebased since my previous review, verified with

$ git range-diff master ffc8ff57f0211603da7f0a1a9af5ef6fafaa5dd6 92993aa5cf37995e65e68dfd6f129ecaf418e01c

Copy link
Contributor

@klementtan klementtan left a comment

Choose a reason for hiding this comment

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

Code review ACK 92993aa

Copy link
Contributor

@meshcollider meshcollider left a comment

Choose a reason for hiding this comment

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

Code review ACK 92993aa

@meshcollider meshcollider merged commit b1a672d into bitcoin:master Aug 9, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 10, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants