Skip to content

Conversation

@sipa
Copy link
Member

@sipa sipa commented May 9, 2014

No description provided.

@gmaxwell
Copy link
Contributor

gmaxwell commented May 9, 2014

perhaps also move it to the test directory?

@laanwj
Copy link
Member

laanwj commented May 9, 2014

See #4159

@sipa
Copy link
Member Author

sipa commented May 9, 2014

@gmaxwell Done.

@laanwj Dang, double work :)

@laanwj
Copy link
Member

laanwj commented May 9, 2014

Well you also slimmed bignum.h down, so let's go with this one.

@laanwj laanwj mentioned this pull request May 9, 2014
@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/7cd0af7cc222d0694ce72e71458aef460698ee2c for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

@sipa
Copy link
Member Author

sipa commented May 10, 2014

Alternatively, we could replace the scriptnum tests by data-driven ones, and get rid of bignum.h entirely.

Copy link

Choose a reason for hiding this comment

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

Why is this needed here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because of CADDR_TIME_VERSION. It was indirectly included through bignum.h.

@laanwj
Copy link
Member

laanwj commented May 10, 2014

Having bignum in the tests directory is fine for now. At some point we could move to data-driven tests, but it seems low-priority as long as we need OpenSSL for other things.

@laanwj laanwj merged commit 7cd0af7 into bitcoin:master May 10, 2014
laanwj added a commit that referenced this pull request May 10, 2014
7cd0af7 Move bignum.h to test/ (Pieter Wuille)
ccc84e0 Reduce bignum.h now it is only needed for scriptnum_tests (Pieter Wuille)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants