Bitcoin Core Github
44 subscribers
120K links
Download Telegram
🤔 jonatack reviewed a pull request: "docs: Rewrite README to make it more appealing"
(https://github.com/bitcoin/bitcoin/pull/28174#pullrequestreview-1550854157)
A few suggestions.

(I'm not sure who the emojis and "We" narration would be more appealing to and which target audience is being aimed for, but no strong opinion. Sugarcoating things might be a little akin to false advertising, though 😄.)
💬 jonatack commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1276850613)
Use newlines consistently.
💬 jonatack commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1276847563)
"evolving with regular builds" seems awkward and may not be interpreted the same as "regularly built"
💬 jonatack commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1276850279)
```suggestion
We use the https://github.com/bitcoin-core/gui repository solely for GUI development. Its master branch serves as a clone in all monotree repositories. It doesn't have release branches and tags, so you only need to fork it for development purposes.
```

or s/We use/We use the/, s/tags here/tags there/ and s/fork this/fork that/
💬 jonatack commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1276855045)
I like "Please be patient and help out by reviewing and testing" better than "Please understand the delay and assist by testing". The bottleneck aspect might be helpful to keep.
💬 jonatack commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1276856178)
I'm not sure this paragraph is an improvement; it seems a little less easy to read and more verbose.
💬 jonatack commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1276856597)
Not sure this is clearer.
💬 jonatack commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1276859281)
Could also mention the productivity notes in doc/. I like the original text better, however, as the links docs don't contain "all" about contributing.
💬 luke-jr commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#issuecomment-1654647616)
Is there a use case for this, for the typical user? If not, maybe it should be optional and disabled by default?
💬 jonatack commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#issuecomment-1654647719)
Suggest running `test/lint/lint-whitespace.py` on this change to appease the lint CI.
🤔 brunoerg reviewed a pull request: "test, rpc: invalid sighashtype coverage"
(https://github.com/bitcoin/bitcoin/pull/28166#pullrequestreview-1550910355)
light crACK 90c8f79e945863f3818748b86572948d1558aec3
💬 aureleoules commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#issuecomment-1654657160)
Thanks @jonatack, I addressed your suggestions and rolled back some of my changes.
💬 jonatack commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1276879626)
FWIW it looks like this section dates back to 2012 or earlier!

```
Testing and code review is the bottleneck for development; we get more
pull requests than we can review and test. Please be patient and help
out, and remember this is a security-critical project where any
mistake might cost people lots of money.
```
💬 luke-jr commented on pull request "Silent Payments: send and receive":
(https://github.com/bitcoin/bitcoin/pull/27827#issuecomment-1654676047)
>Can you explain your reasoning for wanting to split it up? I'd prefer to keep them together only because I see no reason to merge sending without receiving support in Bitcoin Core.

1. The sooner sending is supported everywhere, the sooner it is *practical* for people to use it for receiving. Might as well get that ball rolling ASAP.
2. Sending support means the BIP isn't going to change under me (and break compatibility / risk coin loss) if I merge things in Knots first.
3. Core refuses t
...
🤔 jonatack reviewed a pull request: "docs: Rewrite README to make it more appealing"
(https://github.com/bitcoin/bitcoin/pull/28174#pullrequestreview-1550942991)
A few more suggestions.

Just one opinion: in dark mode, which I use exclusively, I find the emojis are large and highly distracting from the text. If others agree, maybe remove them or make them smaller.
💬 jonatack commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1276887186)
```suggestion
We use the https://github.com/bitcoin-core/gui repository solely for GUI development. Its master branch serves as a clone in all monotree repositories. It doesn't have release branches and tags, so it's only useful to fork it for development purposes.
```
or just s/tags there,/tags,/
💬 jonatack commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1276886210)
```suggestion
Curious to know more? Find further details in [doc folder](/doc).
```
💬 jonatack commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1276888844)
Suggest dropping either "Important:" or "Please note," -- both seem to be too much.
💬 luke-jr commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#discussion_r1276896030)
We don't typically refer to "Bitcoind" like this
💬 luke-jr commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#discussion_r1276895712)
Probably should be called `-stratumv2port`, unless it supports specifying an IP to bind to (probably a good idea)