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

READY: Chain.com PROVIDER added + minor bug fix #86

Closed
wants to merge 7 commits into from
Closed

READY: Chain.com PROVIDER added + minor bug fix #86

wants to merge 7 commits into from

Conversation

shayanb
Copy link
Contributor

@shayanb shayanb commented Feb 23, 2015

fixing #85

@posita
Copy link
Contributor

posita commented Feb 23, 2015

👍 Thanks @shayanb!

EDIT: @mbogosian is now @posita.

@shayanb
Copy link
Contributor Author

shayanb commented Feb 27, 2015

@mbogosian I can't find where it is included, anyhow it wasn't intended to be included, that is just a test file I'm using for playing around the library.

@posita
Copy link
Contributor

posita commented Feb 27, 2015

@shayanb, sorry to beat a dead horse 🐴. This might expose my complete ignorance of how Git or GitHub work, but I was referring to commit 6da5b8a (fix BIP32 for testnet) referenced after my #86 (comment). It looks like that includes the modifications to .gitignore that I mentioned. If this is not the case, my apologies. I have precious little experience with cross-repository pull requests. 😊

Edit: I also see the changes to .gitignore under the files for this PR. The way I understand cross-repository pull requests to work is that all changes committed to the merge-from branch after the PR is created will show up in the PR. So if you create a PR from the master branch of your fork, any subsequent commits you make to your own master branch will also show up in the PR until that PR is closed. I was recently exposed to this behavior when I committed my fix to #83 to my own master branch (see #84), but then wanted to submit another PR for #89. In the end, I created two branches from the master branch of my fork to deal with each separately without polluting each other. Then I created PRs from those branches (see #88 and #90, respectively). My apologies if this is remedial. It's relatively new to me, since I don't do many of these.

@shayanb shayanb changed the title minor bug fix READY: Chain.com PROVIDER added + minor bug fix Mar 25, 2015
@richardkiss
Copy link
Owner

This pull request has a lot going on in it. Some of the fixes have already been applied. @shayanb I'm going to close this request... if there are any changes that you think you need that aren't yet merged, perhaps you can do them one branch and change at a time so the discussion can be more focussed.

@shayanb
Copy link
Contributor Author

shayanb commented Mar 26, 2015

@richardkiss sure, Thanks.
I will send another PR for chain.com provider. not sure the rest of the changes would be applicable anymore.

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.

3 participants