rkrux

Results 28 comments of rkrux
trafficstars

I'm reviewing the PR again and will finalise my review(s) in the coming days.

https://github.com/bitcoin/bitcoin/pull/32116#issuecomment-2754401102 contains good points to consider. > This isn't a lot of code, and it's very well-defined code that isn't likely to change over time. Making the script depend on...

One reason I find deduplicating here appealing is that these (de)compress/deserialize utility functions have quite a custom implementation with lack of comments/explanation. My preference would be to have them present...

Closing this one now.

I did the TODO mentioned in the PR description in PR #32866.

> Concept ACK [d60b008](https://github.com/bitcoin/bitcoin/commit/d60b0088e81e2ef81b9cfe2d4b808ed819a52962) It's a good refactoring for understanding, but perhaps you should still check and modify other documents to make sure it would not missing the explanation of...

Rebased over master to incorporate changes from #31244.

True. The main reason for this change was that I didn't see any benefit in leaving ALL around in the function documentation when the implementation uses DEFAULT. I am open...

@fjahr Thanks for raising it, I'll add it.

@monlovesmango Thanks for following the guide and dsharing your feedback. I've gone through it and the points you shared are correct, hence, I've updated the guide.