🤔 w0xlt reviewed a pull request: "doc: unify `datacarriersize` warning with release notes"
(https://github.com/bitcoin/bitcoin/pull/33224#pullrequestreview-3149210453)
ACK https://github.com/bitcoin/bitcoin/pull/33224/commits/2885bd0e1c4fc863a7f28ff0fd353f5cffb03442
Will the `src/qt/bitcoinstrings.cpp` file be changed in the GUI repository?
(https://github.com/bitcoin/bitcoin/pull/33224#pullrequestreview-3149210453)
ACK https://github.com/bitcoin/bitcoin/pull/33224/commits/2885bd0e1c4fc863a7f28ff0fd353f5cffb03442
Will the `src/qt/bitcoinstrings.cpp` file be changed in the GUI repository?
🤔 hebasto reviewed a pull request: "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings"
(https://github.com/bitcoin/bitcoin/pull/33247#pullrequestreview-3149213360)
> This is my first attempt at contributing to bitcoin core so please, provide me any feedback that you might have. Thanks!
Thank you for your interest in contributing to this project!
The changes you are referring to as completed, i.e. "reordering Guix script commands to perform binary checks after the installation step", have not actually been done yet. See [`contrib/guix/libexec/build.sh`](https://github.com/bitcoin/bitcoin/blob/master/contrib/guix/libexec/build.sh), from line 251 onward
...
(https://github.com/bitcoin/bitcoin/pull/33247#pullrequestreview-3149213360)
> This is my first attempt at contributing to bitcoin core so please, provide me any feedback that you might have. Thanks!
Thank you for your interest in contributing to this project!
The changes you are referring to as completed, i.e. "reordering Guix script commands to perform binary checks after the installation step", have not actually been done yet. See [`contrib/guix/libexec/build.sh`](https://github.com/bitcoin/bitcoin/blob/master/contrib/guix/libexec/build.sh), from line 251 onward
...
👍 hebasto approved a pull request: "Update libmultiprocess subtree to fix build issues"
(https://github.com/bitcoin/bitcoin/pull/33241#pullrequestreview-3149218774)
ACK 323b3fd27283282f2f8eb1096f56f23d8230f2d6, I've reproduced the subtree update locally. The two issues noted in this PR are unrelated to its changes and can be addressed separately.
(https://github.com/bitcoin/bitcoin/pull/33241#pullrequestreview-3149218774)
ACK 323b3fd27283282f2f8eb1096f56f23d8230f2d6, I've reproduced the subtree update locally. The two issues noted in this PR are unrelated to its changes and can be addressed separately.
💬 maflcko commented on issue "`feature_bind_extra.py` test fails in `test_runner` if new nodes are added":
(https://github.com/bitcoin/bitcoin/issues/33250#issuecomment-3217960830)
There is a limit on the number of nodes `MAX_NODES` and ports that can be used per test. You'll have to stay under it, or increase the limit.
(https://github.com/bitcoin/bitcoin/issues/33250#issuecomment-3217960830)
There is a limit on the number of nodes `MAX_NODES` and ports that can be used per test. You'll have to stay under it, or increase the limit.
💬 IdotMaster1 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-3217994772)
Concept NACK.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-3217994772)
Concept NACK.
💬 hebasto commented on pull request "doc: Replace FIXME placeholders with correct libblkmaker version numbers":
(https://github.com/bitcoin/bitcoin/pull/33248#issuecomment-3218035869)
> ... it looks like the pull request was closed automatically. Can someone help me understand why...
See https://github.com/bitcoin/bitcoin/pull/33248#issuecomment-3217454645.
(https://github.com/bitcoin/bitcoin/pull/33248#issuecomment-3218035869)
> ... it looks like the pull request was closed automatically. Can someone help me understand why...
See https://github.com/bitcoin/bitcoin/pull/33248#issuecomment-3217454645.
💬 hebasto commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3218109881)
> cc @hebasto; given this would change translations after translation string freeze.
The translation workflow on Transifex includes marking translated strings as "Reviewed", which locks them from further changes. Not every translation team uses this feature, but those who do rely on it. Unfortunately, the "opensource" plan used by the [Bitcoin organization](https://explore.transifex.com/bitcoin/) on Transifex has very limited functionality, and it is not guaranteed that updating the translati
...
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3218109881)
> cc @hebasto; given this would change translations after translation string freeze.
The translation workflow on Transifex includes marking translated strings as "Reviewed", which locks them from further changes. Not every translation team uses this feature, but those who do rely on it. Unfortunately, the "opensource" plan used by the [Bitcoin organization](https://explore.transifex.com/bitcoin/) on Transifex has very limited functionality, and it is not guaranteed that updating the translati
...
💬 hebasto commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3218111653)
@w0xlt
> Will the `src/qt/bitcoinstrings.cpp` file be changed in the GUI repository?
It makes no difference.
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3218111653)
@w0xlt
> Will the `src/qt/bitcoinstrings.cpp` file be changed in the GUI repository?
It makes no difference.
💬 hebasto commented on pull request "[29.x] depends: remove xinerama extension from libxcb":
(https://github.com/bitcoin/bitcoin/pull/33238#issuecomment-3218125089)
> Guess we can't backport this unless we also backport the removal in Qt. Will take a look.
Indeed. And there's no `configure` option to disable the XCB Xinerama extension.
I assume that backporting https://github.com/qt/qtbase/commit/c91d1fdc100cda88b94217153def52ab7fe63d21 might be non-trivial, and it would definitely require extensive testing across a range of platforms.
While the dependency on this extension ["is annoying/confusing for users"](https://github.com/bitcoin/bitcoin/pull
...
(https://github.com/bitcoin/bitcoin/pull/33238#issuecomment-3218125089)
> Guess we can't backport this unless we also backport the removal in Qt. Will take a look.
Indeed. And there's no `configure` option to disable the XCB Xinerama extension.
I assume that backporting https://github.com/qt/qtbase/commit/c91d1fdc100cda88b94217153def52ab7fe63d21 might be non-trivial, and it would definitely require extensive testing across a range of platforms.
While the dependency on this extension ["is annoying/confusing for users"](https://github.com/bitcoin/bitcoin/pull
...
💬 151henry151 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3218145102)
> > This is my first attempt at contributing to bitcoin core so please, provide me any feedback that you might have. Thanks!
>
> Thank you for your interest in contributing to this project!
>
> The changes you are referring to as completed, i.e. "reordering Guix script commands to perform binary checks after the installation step", have not actually been done yet. See [`contrib/guix/libexec/build.sh`](https://github.com/bitcoin/bitcoin/blob/master/contrib/guix/libexec/build.sh), from line
...
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3218145102)
> > This is my first attempt at contributing to bitcoin core so please, provide me any feedback that you might have. Thanks!
>
> Thank you for your interest in contributing to this project!
>
> The changes you are referring to as completed, i.e. "reordering Guix script commands to perform binary checks after the installation step", have not actually been done yet. See [`contrib/guix/libexec/build.sh`](https://github.com/bitcoin/bitcoin/blob/master/contrib/guix/libexec/build.sh), from line
...
🤔 hebasto reviewed a pull request: "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings"
(https://github.com/bitcoin/bitcoin/pull/33247#pullrequestreview-3149363243)
In a02723427f5f0b142a6ef97b92df093e00ad6004, all checks are still performed on the binaries in the build tree, whereas they are supposed to be performed on the binaries in the installation location.
(https://github.com/bitcoin/bitcoin/pull/33247#pullrequestreview-3149363243)
In a02723427f5f0b142a6ef97b92df093e00ad6004, all checks are still performed on the binaries in the build tree, whereas they are supposed to be performed on the binaries in the installation location.
💬 hebasto commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3218149480)
> ... I tested it locally and it seemed to build successfully...
When a developer tests changes in the Guix scripts locally, they usually posts the resulting binary hashes. For example, as in https://github.com/bitcoin/bitcoin/pull/33217#pullrequestreview-3142128074.
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3218149480)
> ... I tested it locally and it seemed to build successfully...
When a developer tests changes in the Guix scripts locally, they usually posts the resulting binary hashes. For example, as in https://github.com/bitcoin/bitcoin/pull/33217#pullrequestreview-3142128074.
💬 151henry151 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3218151904)
> ### Conflicts
>
> Reviewers, this pull request conflicts with the following ones:
>
> * [#32380](https://github.com/bitcoin/bitcoin/pull/32380) (Modernize use of UTF-8 in Windows code by hebasto)
>
The conflict resolves easily by keeping both changes.
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3218151904)
> ### Conflicts
>
> Reviewers, this pull request conflicts with the following ones:
>
> * [#32380](https://github.com/bitcoin/bitcoin/pull/32380) (Modernize use of UTF-8 in Windows code by hebasto)
>
The conflict resolves easily by keeping both changes.
💬 l0rinc commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3218160937)
@hebasto, how can we help with finding a solution?
I personally would be okay with only fixing the English version, if updating the rest is indeed an unsolvable problem.
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3218160937)
@hebasto, how can we help with finding a solution?
I personally would be okay with only fixing the English version, if updating the rest is indeed an unsolvable problem.
💬 janb84 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3218182511)
> This is my first attempt at contributing to bitcoin core so please, provide me any feedback that you might have. Thanks!
Welcome, please also change this message for a description of the changes made in this PR. Helpful tips are in [creating-the-pull-request](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#creating-the-pull-request)
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3218182511)
> This is my first attempt at contributing to bitcoin core so please, provide me any feedback that you might have. Thanks!
Welcome, please also change this message for a description of the changes made in this PR. Helpful tips are in [creating-the-pull-request](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#creating-the-pull-request)
💬 151henry151 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3218186464)
> In [a027234](https://github.com/bitcoin/bitcoin/commit/a02723427f5f0b142a6ef97b92df093e00ad6004), all checks are still performed on the binaries in the build tree, whereas they are supposed to be performed on the binaries in the installation location.
Thanks for the feedback.
The previous change moved checks after installation but was still using cmake --build targets which check build tree binaries. I've now fixed this to directly call the security and symbol check scripts on the instal
...
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3218186464)
> In [a027234](https://github.com/bitcoin/bitcoin/commit/a02723427f5f0b142a6ef97b92df093e00ad6004), all checks are still performed on the binaries in the build tree, whereas they are supposed to be performed on the binaries in the installation location.
Thanks for the feedback.
The previous change moved checks after installation but was still using cmake --build targets which check build tree binaries. I've now fixed this to directly call the security and symbol check scripts on the instal
...
💬 surajoibirahim143-ops commented on something "":
(https://github.com/bitcoin/bitcoin/commit/9703b7e6d563ea58f83a6eca819562365404f4ab#commitcomment-164519142)
10000
(https://github.com/bitcoin/bitcoin/commit/9703b7e6d563ea58f83a6eca819562365404f4ab#commitcomment-164519142)
10000
📝 fjahr opened a pull request: "[29.x] backport #33212"
(https://github.com/bitcoin/bitcoin/pull/33251)
Backports #33212 to 29.x
(https://github.com/bitcoin/bitcoin/pull/33251)
Backports #33212 to 29.x
💬 fanquake commented on pull request "[29.x] backport #33212":
(https://github.com/bitcoin/bitcoin/pull/33251#issuecomment-3218395662)
Can you add the GitHub-Pull / Rebased-From metadata to each commit.
(https://github.com/bitcoin/bitcoin/pull/33251#issuecomment-3218395662)
Can you add the GitHub-Pull / Rebased-From metadata to each commit.
💬 fanquake commented on pull request "index: Don't commit state in BaseIndex::Rewind":
(https://github.com/bitcoin/bitcoin/pull/33212#issuecomment-3218397979)
Backported to 29.x in 33251.
(https://github.com/bitcoin/bitcoin/pull/33212#issuecomment-3218397979)
Backported to 29.x in 33251.