Tests: remove bignum module #17319

pull jnewbery wants to merge 7 commits into bitcoin:master from jnewbery:2019-10-bignum changing 3 files +173 −197
  1. jnewbery commented at 2:58 pm on October 30, 2019: member

    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.

  2. [tests] fix flake8 warnings in script.py and bignum.py f31fc0e92e
  3. [tests] add function comments to bignum 1dc68aee66
  4. [tests] don't encode the integer size in bignum
    We just throw it away whenever we use the
    result so don't add it.
    9a60bef50d
  5. [tests] remove mpi2vch() function
    All it does is reverse the bytes order.
    a760aa14a9
  6. [tests] remove bn_bytes() function
    It is one line and is called in one place.
    3b9b38579c
  7. [tests] remove bn2bin()
    It's only called in one place.
    f950ec2520
  8. fanquake added the label Tests on Oct 30, 2019
  9. laanwj commented at 3:01 pm on October 30, 2019: member
    I’m confused by the commit order here. First you add comments to bignum, then you remove the whole module.
  10. in test/functional/test_framework/script.py:12 in 60fcb35050 outdated
    10-from .messages import CTransaction, CTxOut, sha256, hash256, uint256_from_str, ser_uint256, ser_string
    11-
    12 import hashlib
    13 import struct
    14 
    15 from .bignum import bn2vch
    


    laanwj commented at 3:01 pm on October 30, 2019:
    This import should go, I guess?

    jnewbery commented at 3:04 pm on October 30, 2019:
    yes it should! Fixed
  11. jnewbery commented at 3:02 pm on October 30, 2019: member

    I’m confused by the commit order here. First you add comments to bignum, then you remove the whole module.

    Comments are for the benefit of reviewers so they can see what functionality is being moved around.

  12. [tests] remove bignum.py
    It only contains one function and is only imported by one
    other module (script.py). Just move the function to
    script.py.
    3ed772d221
  13. jnewbery force-pushed on Oct 30, 2019
  14. laanwj commented at 3:03 pm on October 30, 2019: member

    Comments are for the benefit of reviewers so they can see what functionality is being moved around.

    I’d be ok with keeping the bignum module, just removing the functions that are no longer used. Not sure script.py is a much better place for that function.

  15. practicalswift commented at 4:14 pm on October 30, 2019: contributor
    Concept ACK on removing functions that are no longer used or tested
  16. fanquake commented at 10:52 am on January 4, 2020: member
    Concept ACK
  17. fanquake requested review from MarcoFalke on Jan 4, 2020
  18. DrahtBot commented at 1:14 am on January 22, 2020: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #18210 (test: type hints in Python tests by kiminuo)

    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.

  19. sipa commented at 11:46 pm on March 13, 2020: member
    Concept ACK
  20. MarcoFalke merged this on Mar 17, 2020
  21. MarcoFalke closed this on Mar 17, 2020

  22. jnewbery deleted the branch on Mar 17, 2020
  23. sidhujag referenced this in commit fff9415557 on Mar 18, 2020
  24. sidhujag referenced this in commit 6c97cd0637 on Nov 10, 2020
  25. Fabcien referenced this in commit a5d817fae8 on Jan 9, 2021
  26. DrahtBot locked this on Feb 15, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-24 09:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me