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

refactor: get wallet path relative to wallet_dir #23385

Merged
merged 1 commit into from Oct 30, 2021

Conversation

mjdietzx
Copy link
Contributor

@mjdietzx mjdietzx commented Oct 29, 2021

Now that boost has been updated > 1.60 (see #22320), we can simplify how we get
wallet path relative to wallet_dir by using:
boost::filesystem::lexically_relative, removing a TODO.

Test coverage comes from test/functional/wallet_multiwallet.py

I first tried this in #20265 which was my first attempted PR, and funny enough exactly 1 year later I'm opening this one to hopefully finally close this.

Now that boost has been updated > 1.60, we can simplify how we get
wallet path relative to wallet_dir by using:
`boost::filesystem::lexically_relative`
Copy link
Member

@promag promag left a comment

Choose a reason for hiding this comment

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

Concept ACK, thanks!

🎂 1 yeah 🤣

@ryanofsky
Copy link
Contributor

Comment #23385 (comment) appears to be a spam comment pasting PR description from #22320

Copy link
Contributor

@ryanofsky ryanofsky 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 9ba7c44. Basically this same code change is made in #20744 commit b70c843, so this PR helps simplify that one

@bitcoin bitcoin deleted a comment Oct 30, 2021
@DrahtBot
Copy link
Contributor

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #20744 (Use std::filesystem. Remove Boost Filesystem & System by fanquake)

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
Contributor

@lsilva01 lsilva01 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 9ba7c44

@fanquake fanquake merged commit 7efc628 into bitcoin:master Oct 30, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 30, 2021
9ba7c44 refactor: get wallet path relative to wallet_dir (Michael Dietz)

Pull request description:

  Now that boost has been updated > 1.60 (see bitcoin#22320), we can simplify how we get
  wallet path relative to wallet_dir by using:
  `boost::filesystem::lexically_relative`, removing a TODO.

  Test coverage comes from `test/functional/wallet_multiwallet.py`

  I first tried this in bitcoin#20265 which was my first attempted PR, and funny enough exactly 1 year later I'm opening this one to hopefully finally close this.

ACKs for top commit:
  ryanofsky:
    Code review ACK 9ba7c44. Basically this same code change is made in bitcoin#20744 commit b70c843, so this PR helps simplify that one
  lsilva01:
    Code Review ACK 9ba7c44

Tree-SHA512: 6ccb91a18bcb52c3ae0c789a94a18fb5be7db7769fd1121552d63f259fbd32b50c3dcf169cec0b02f978321db3bc60eb4b881b8327e9764f32e700236e0d8a35
@bitcoin bitcoin locked and limited conversation to collaborators Oct 30, 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.

None yet

6 participants