👍 hebasto approved a pull request: "util: remove unused cpp-subprocess options"
(https://github.com/bitcoin/bitcoin/pull/29865#pullrequestreview-2017207809)
re-ACK 899be1a7643040c32510903dab06cea5ef7a5ffd.

💬 hebasto commented on pull request "util: remove unused cpp-subprocess options":
(https://github.com/bitcoin/bitcoin/pull/29865#discussion_r1576227708)
nit: Maybe a wording more consistent with other comment around:
```suggestion
NULL, // use parent's environment
```

💬 theStack commented on pull request "util: remove unused cpp-subprocess options":
(https://github.com/bitcoin/bitcoin/pull/29865#issuecomment-2072269247)
Force-pushed with the last commit changed to tackle https://github.com/bitcoin/bitcoin/pull/29865#discussion_r1571141464.

<details>
<summary>Diff to the previous version</summary>

```diff
diff --git a/src/util/subprocess.hpp b/src/util/subprocess.hpp
index c7f9a685d1..a85c0b0bff 100644
--- a/src/util/subprocess.hpp
+++ b/src/util/subprocess.hpp
@@ -1210,8 +1210,6 @@ inline void Popen::k
...

👍 hebasto approved a pull request: "util: remove unused cpp-subprocess options"
(https://github.com/bitcoin/bitcoin/pull/29865#pullrequestreview-2017224404)
re-ACK 13adbf733f09c73c3cf0025d94c52f9cec5dba3b.

💬 theStack commented on pull request "util: remove unused cpp-subprocess options":
(https://github.com/bitcoin/bitcoin/pull/29865#discussion_r1576236387)
Thanks, done.

💬 Umiiii commented on pull request "test(mempool): add missing comparison of node1's mempool in MempoolPackagesTest":
(https://github.com/bitcoin/bitcoin/pull/29940#issuecomment-2072299641)
Believe the CI failed for lint problems (trailing whitespace and blank line contains whitespace), I've tested the code locally and passed the regression test.

💬 hebasto commented on pull request "util: remove unused cpp-subprocess options":
(https://github.com/bitcoin/bitcoin/pull/29865#issuecomment-2072308000)
> This seems likely to have a high maintenance cost.

As discussed in https://github.com/bitcoin/bitcoin/pull/28981 and during the IRC [meeting](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-12-07#988888;), we are going to treat new code as our own:
> [14:16](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-12-07#988909) \<fanquake\> Obviously having a static header in our repo is an improvement, similar to nanobench/tinyformat

💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1576261445)
> Is the suggestion to drop this and only try same-peer packages?

If this is hitting actual usage, no, because it's coded and I actually reviewed it, and tests seem to cover it. I'm unsure if in BIP331-like world it would be used since it would be receiver-driven, but that might be thinking too far ahead.

> (In this scenario I guess I'm assuming no feefilter, or that the feefilter value is slightly stale.)

💬 hebasto commented on pull request "Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-2072324885)
> ... with the expectation that this code is going to be maintained as our own. Next PRs should:
>
> * Remove linter exclusions and fix all issues.

See:
- https://github.com/bitcoin/bitcoin/pull/29849
- https://github.com/bitcoin/bitcoin/pull/29910

> * Delete all unused code.

The work has been started in https://github.com/bitcoin/bitcoin/pull/29865.

✅ Umiiii closed a pull request: "test: add missing comparison of node1's mempool in MempoolPackagesTest"
(https://github.com/bitcoin/bitcoin/pull/29940)

📝 Umiiii opened a pull request: "test: add missing comparison of node1's mempool in MempoolPackagesTest"
(https://github.com/bitcoin/bitcoin/pull/29941)
Add missing comparison for TODO comments in `mempool_packages.py`

Also, notice that the ancestor size limits and descendant size limits actually implemented in #21800 , so I removed the todo for those two size limits.

🤔 glozow reviewed a pull request: "test: add missing comparison of node1's mempool in MempoolPackagesTest"
(https://github.com/bitcoin/bitcoin/pull/29941#pullrequestreview-2017352438)
Thanks for the pull, please [squash your commits](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits)

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576316804)
done

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576316974)
done

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576317113)
done

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576317267)
both done

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576317764)
done, I took this naming from libevent exactly fwiw but since we change the behavior and tests anyway, it's not that relevant anymore to keep it consistent

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2072397502)
Addressed latest feedback from @hebasto and @stickies-v , thank you!

💬 hebasto commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576323273)
Indentation?

💬 Umiiii commented on pull request "test: add missing comparison of node1's mempool in MempoolPackagesTest":
(https://github.com/bitcoin/bitcoin/pull/29941#issuecomment-2072406459)
> Thanks for the pull, please [squash your commits](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits)

squashed.