Skip to content
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

test: Replace hashlib.new with named constructor #23723

Merged
merged 1 commit into from
Dec 9, 2021
Merged

test: Replace hashlib.new with named constructor #23723

merged 1 commit into from
Dec 9, 2021

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Dec 9, 2021

A small refactor that doesn't matter too much, but it using the named constructor is nice because:

  • It clarifies that it is a built-in function
  • It is (trivially) faster and less code.

@maflcko
Copy link
Member Author

maflcko commented Dec 9, 2021

This removes the last use of new: git grep 'hashlib.new'.

@fanquake fanquake added the Tests label Dec 9, 2021
Zero-1729
Zero-1729 reviewed Dec 9, 2021
View reviewed changes
Copy link
Contributor

@Zero-1729 Zero-1729 left a comment

Choose a reason for hiding this comment

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

ACK fa1b63c

Verified no other instance of hashlib.new after this patch.

w0xlt
w0xlt approved these changes Dec 9, 2021
View reviewed changes
Copy link
Contributor

@w0xlt w0xlt left a comment

Choose a reason for hiding this comment

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

ACK fa1b63c

@maflcko maflcko merged commit 09e60df into bitcoin:master Dec 9, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 10, 2021
Merge bitcoin#23723: test: Replace hashlib.new with named constructor
777d236 
fa1b63c test: Replace hashlib.new with named constructor (MarcoFalke)

Pull request description:

  A small refactor that doesn't matter too much, but it using the named constructor is nice because:
  * It clarifies that it is a built-in function
  * It is (trivially) faster and less code.

ACKs for top commit:
  Zero-1729:
    ACK fa1b63c
  w0xlt:
    ACK fa1b63c

Tree-SHA512: d23dc4552c1e6fc1f90f8272e47e4efcbe727f0b66a6f6a264db8a50ee6cb6d57a2809befcb95fda6725136672268633817a03dd1859f2298d20e3f9e0ca4a7f
RandyMcMillan pushed a commit to RandyMcMillan/mempool-tab that referenced this pull request Dec 23, 2021
Merge bitcoin/bitcoin#23723: test: Replace hashlib.new with named con…
4c68f61 
…structor

ae2c424 test: Replace hashlib.new with named constructor (MarcoFalke)

Pull request description:

  A small refactor that doesn't matter too much, but it using the named constructor is nice because:
  * It clarifies that it is a built-in function
  * It is (trivially) faster and less code.

ACKs for top commit:
  Zero-1729:
    ACK ae2c424
  w0xlt:
    ACK ae2c424

Tree-SHA512: d23dc4552c1e6fc1f90f8272e47e4efcbe727f0b66a6f6a264db8a50ee6cb6d57a2809befcb95fda6725136672268633817a03dd1859f2298d20e3f9e0ca4a7f
@bitcoin bitcoin locked and limited conversation to collaborators Dec 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Zero-1729 Zero-1729 Zero-1729 left review comments

@w0xlt w0xlt w0xlt approved these changes

Assignees
No one assigned
Labels
Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@maflcko @Zero-1729 @w0xlt @fanquake