8000 Remove deprecated `StreamReader` by Kixunil · Pull Request #1144 · rust-bitcoin/rust-bitcoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Remove deprecated StreamReader #1144

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 1, 2022
Merged

Remove deprecated StreamReader #1144

merged 1 commit into from
Aug 1, 2022

Conversation

Kixunil
Copy link
Collaborator
@Kixunil Kixunil commented Jul 28, 2022

StreamReader was deprecated for a while, yet we needlessly maintain it
and it eats our testing time. This change removes it completely.

Closes #1123

@Kixunil Kixunil added API break This PR requires a version bump for the next release code quality Makes code easier to understand and less likely to lead to problems trivial Obvious, easy and quick to review (few lines or doc-only...) labels Jul 28, 2022
@Kixunil Kixunil force-pushed the remove-stream-reader branch 3 times, most recently from ac65947 to 8d7f4cb Compare July 30, 2022 12:49
@Kixunil
Remove deprecated StreamReader
0c9c141 
`StreamReader` was deprecated for a while, yet we needlessly maintain it
and it eats our testing time. This change removes it completely.

Closes rust-bitcoin#1123
@Kixunil Kixunil force-pushed the remove-stream-reader branch from 8d7f4cb to 0c9c141 Compare July 30, 2022 12:59
tcharding
tcharding approved these changes Aug 1, 2022
View reviewed changes
Copy link
Member
@tcharding tcharding left a comment •
edited
Loading

Choose a reason for hiding this comment

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

ACK 0c9c141 (with the function rename).

src/blockdata/block.rs
Copy link
Member

Choose a reason for hiding this comment

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

I haven't groked the bench mark code in general but this function probably should be renamed to say what it is bench marking, possibly bench_block_deserialize_logic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, I didn't read the function name. 😅 I will have to look if it's even needed, maybe it should be removed.

Copy link
Member

Choose a reason for hiding this comment

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

Cheers, I could not work out if it was adding value or not.

Copy link
Member

Choose a reason for hiding this comment

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

My intuition is we should probably drop it. Can do this in a followup PR

@Kixunil Kixunil mentioned this pull request Aug 1, 2022
Merged
3 tasks
sanket1729
sanket1729 approved these changes Aug 1, 2022
View reviewed changes
Copy link
Member
@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

utACK 0c9c141.

apoelstra
apoelstra approved these changes Aug 1, 2022
View reviewed changes
Copy link
Member
@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 0c9c141

@apoelstra apoelstra merged commit ef5014f into rust-bitcoin:master Aug 1, 2022
@Kixunil Kixunil deleted the remove-stream-reader branch August 1, 2022 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apoelstra apoelstra apoelstra approved these changes

@tcharding tcharding tcharding approved these changes

@sanket1729 sanket1729 sanket1729 approved these changes

Assignees
No one assigned
Labels
API break This PR requires a version bump for the next release code quality Makes code easier to understand and less likely to lead to problems trivial Obvious, easy and quick to review (few lines or doc-only...)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Useless TCP test
4 participants
@Kixunil @apoelstra @sanket1729 @tcharding
0
let mut reader = StreamReader::new(&big_block[..], None);
let block: Block = reader.read_next().unwrap();
let mut reader = &big_block[..];
let block = Block::consensus_decode(&mut reader).unwrap();