Tests: remove bignum module #17319
Merged
Tests: remove bignum module #17319
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We just throw it away whenever we use the result so don't add it.
All it does is reverse the bytes order.
It is one line and is called in one place.
It's only called in one place.
Member
|
I'm confused by the commit order here. First you add comments to bignum, then you remove the whole module. |
laanwj
reviewed
Oct 30, 2019
Contributor
Author
Comments are for the benefit of reviewers so they can see what functionality is being moved around. |
It only contains one function and is only imported by one other module (script.py). Just move the function to script.py.
jnewbery
force-pushed
the
2019-10-bignum
branch
from
60fcb35 to
3ed772d
Compare
Member
I'd be ok with keeping the bignum module, just removing the functions that are no longer used. |
Contributor
|
Concept ACK on removing functions that are no longer used or tested |
Member
|
Concept ACK |
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
4 tasks
Member
|
Concept ACK |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Mar 18, 2020
3ed772d [tests] remove bignum.py (John Newbery) f950ec2 [tests] remove bn2bin() (John Newbery) 3b9b385 [tests] remove bn_bytes() function (John Newbery) a760aa1 [tests] remove mpi2vch() function (John Newbery) 9a60bef [tests] don't encode the integer size in bignum (John Newbery) 1dc68ae [tests] add function comments to bignum (John Newbery) f31fc0e [tests] fix flake8 warnings in script.py and bignum.py (John Newbery) Pull request description: Only one function is imported in script.py. Just move that function to script.py and remove the bignum.py module. Remove unused functionality and fix some flake8 warnings along the way. Top commit has no ACKs. Tree-SHA512: 015f543ab545b5d5451896e2751d9c19334d9155b03faacd2023781e89833a2440f7f28741e9a8ac49badd9cdc012cbb6e038cdcdebeefaf9cb9d461c0689157
sidhujag
pushed a commit
to syscoin-core/syscoin
that referenced
this pull request
Nov 10, 2020
3ed772d [tests] remove bignum.py (John Newbery) f950ec2 [tests] remove bn2bin() (John Newbery) 3b9b385 [tests] remove bn_bytes() function (John Newbery) a760aa1 [tests] remove mpi2vch() function (John Newbery) 9a60bef [tests] don't encode the integer size in bignum (John Newbery) 1dc68ae [tests] add function comments to bignum (John Newbery) f31fc0e [tests] fix flake8 warnings in script.py and bignum.py (John Newbery) Pull request description: Only one function is imported in script.py. Just move that function to script.py and remove the bignum.py module. Remove unused functionality and fix some flake8 warnings along the way. Top commit has no ACKs. Tree-SHA512: 015f543ab545b5d5451896e2751d9c19334d9155b03faacd2023781e89833a2440f7f28741e9a8ac49badd9cdc012cbb6e038cdcdebeefaf9cb9d461c0689157
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jan 9, 2021
Summary: The helper module bignum.py in the functional test framework provides only one function that is used outside the module (`bn2vch`) and it is used only in script.py, Remove `bignum.py` and move the function to `script.py` PR17319 introduced a bug, so I had to merge also a commit from PR18378 which rewrites entirely the function `bn2vch` This is a backport of Core [[bitcoin/bitcoin#17319 | PR17319]] Additional bugfix commit: bitcoin/bitcoin@a3ad645 The rest of PR18378 will be in a separate diff. Test Plan: `ninja check-functional-extended` Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D8852
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Only one function is imported in script.py. Just move that function to script.py and remove the bignum.py module.
Remove unused functionality and fix some flake8 warnings along the way.