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

Add simple orderbook merging from updates #627

Merged
merged 3 commits into from
Aug 15, 2021
Merged

Conversation

jdx-john
Copy link
Contributor

Kraken definitely uses this partial orderbook update format (volume==0 means deletion) and I think Binance does too, so it seems like a useful utility method.

(again, my fork has got messed up and insists it is ahead of yours. It is telling me the last PR which you accepted still needs to be committed even though I updated my repo from upstream before making changes. )

Copy link
Collaborator

@vslee vslee left a comment

Choose a reason for hiding this comment

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

(again, my fork has got messed up and insists it is ahead of yours. It is telling me the last PR which you accepted still needs to be committed even though I updated my repo from upstream before making changes. )

I believe the step you're missing is that after fetching upstream, you then have to reset your current branch to the latest commit. I usually do this using the Hard reset (discarding all local changes). Then I start my work from there. Git Extensions makes this easy, although it is windows only.

@vslee vslee linked an issue Aug 15, 2021 that may be closed by this pull request
@vslee vslee merged commit 6bdffbd into DigitalRuby:master Aug 15, 2021
@jdx-john
Copy link
Contributor Author

(again, my fork has got messed up and insists it is ahead of yours. It is telling me the last PR which you accepted still needs to be committed even though I updated my repo from upstream before making changes. )

I believe the step you're missing is that after fetching upstream, you then have to reset your current branch to the latest commit. I usually do this using the Hard reset (discarding all local changes). Then I start my work from there. Git Extensions makes this easy, although it is windows only.

I don't get why if I submit a PR (on a clean form before I made changes) and then I pull from upstream after it's accepted, my fork isn't in the identical state as your repo. Resetting (or clean fork as many advocate) works but seems to indicate an issue.
I could do everything in branches and submit PRs on them - I would do clean resets but I might start work on the next thing while waiting on the PR and it's bound to happen sometimes!

@vslee
Copy link
Collaborator

vslee commented Aug 17, 2021

I was going to say that there are two copies of the master branch (remote and local). So when you fetch the remote, it only updates the remote copy on your computer, but the local copy that you're working on needs to be set to be the same as the remote copy. But it looks like you already figured that out in your latest PR.
Yes, if there are changes on the remote master before your PR is accepted, then it might require another merge later. Although git is smart enough that that is only needed if there is conflict. So if the change happens in a different file or different portion of the same file, then usually your PR can be accepted and merged without any further commits.

@jdx-john
Copy link
Contributor Author

I was going to say that there are two copies of the master branch (remote and local). So when you fetch the remote, it only updates the remote copy on your computer, but the local copy that you're working on needs to be set to be the same as the remote copy. But it looks like you already figured that out in your latest PR.
Yes, if there are changes on the remote master before your PR is accepted, then it might require another merge later. Although git is smart enough that that is only needed if there is conflict. So if the change happens in a different file or different portion of the same file, then usually your PR can be accepted and merged without any further commits.

I don't think it's that since GitHub makes it so easy to update things but, I decided I would only make changes in new branches in future and submit those as PR. That way I avoid whatever weird situation I'm getting myself into!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is there a sample OrderBook implementation?
2 participants