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.

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

More integration tests #302

Merged
merged 13 commits into from Nov 25, 2014
Merged

More integration tests #302

merged 13 commits into from Nov 25, 2014

Conversation

dcousens
Copy link
Contributor

These are acting as basic integration tests, as well as simplified examples that are easy for users to look at.

I think combined with different efforts in #272, this will make it quicker to get started for users.

@dcousens dcousens added this to the 1.3.0 milestone Oct 19, 2014
@dcousens
Copy link
Contributor Author

Added stealth address example, as per the unsystem wiki.
Only the single-key derivation is shown, but I'll add a dual-key before this is merged.

Might be worth looking to whether we should add an API to make this easy for people as well.

ping @caedesvvv , thoughts?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling 6db700c on inttests into 27e69ba on master.

@weilu
Copy link
Contributor

weilu commented Oct 19, 2014

I like this approach to adding examples, because it prevents example code from rotting. I'm not sure we should use brand names for filenames as it might be seen as an endorsement. The helloblock example is there to illustrate p2sh spending and darkwallet stealth address. Not sure the brainwallet one is necessary as we already have message signing and verification under unit tests.

@dcousens
Copy link
Contributor Author

I agree on the branding not being great. But the purpose was for feature
or example lookup. If you have a more obvious categorization technique,
I'm all for it.
On Oct 19, 2014 5:07 PM, "Wei Lu" notifications@github.com wrote:

I like this approach to adding examples, because it prevents example code
from rotting. I'm not sure we should use brand names for filenames as it
might be seen as an endorsement. The helloblock example is there to
illustrate p2sh spending and darkwallet stealth address. Not sure the
brainwallet one is necessary as we already have message signing and
verification under unit tests.


Reply to this email directly or view it on GitHub
#302 (comment)
.

@weilu
Copy link
Contributor

weilu commented Oct 19, 2014

helloblock.js -> p2sh.js
darkwallet.js -> stealth_address.js

@dcousens
Copy link
Contributor Author

What about the README/generic examples?

On Sun, Oct 19, 2014 at 5:11 PM, Wei Lu notifications@github.com wrote:

helloblock.js -> p2sh.js
darkwallet.js -> stealth_address.js


Reply to this email directly or view it on GitHub
#302 (comment)
.

@weilu
Copy link
Contributor

weilu commented Oct 19, 2014

Either readme.js stays as is or rename to basic.js. And perhaps instead of code examples on readme we can create the following links to code:

How-To

@dcousens
Copy link
Contributor Author

Confirm the only thing blocking this was the renames?

@weilu
Copy link
Contributor

weilu commented Oct 27, 2014

that and if we want to get rid of brainwallet examples

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d9c716c on inttests into bd8a677 on master.

@dcousens
Copy link
Contributor Author

@weilu should be good to go now.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 7e7071b on inttests into bd8a677 on master.

weilu added a commit that referenced this pull request Nov 25, 2014
@weilu weilu merged commit 5b41ffc into master Nov 25, 2014
@weilu
Copy link
Contributor

weilu commented Nov 25, 2014

@dcousens we might want to pin the links to examples to [sha of next release tag]~2 such that anyone with a release version of the code will have valid example references in the readme.

@dcousens
Copy link
Contributor Author

The example links should remain valid unless someone updates the tests, no? Or did I miss what you were saying?

@dcousens dcousens deleted the inttests branch November 26, 2014 03:28
@dcousens dcousens mentioned this pull request Nov 26, 2014
@weilu
Copy link
Contributor

weilu commented Nov 27, 2014

@dcousens yes. The situation I'm trying avoid is when someone install bitcoinjs-lib using npm, then they navigate to the README in their node_modules, following one of the links which points to master for examples, but the examples has been updated since the version they have. If we do what I described above, they'd end up with links pinned to a sha instead of master.

@dcousens dcousens mentioned this pull request Dec 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants