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

Fixed issue with DSP Dump not closing files. #530

Merged
merged 1 commit into from Jun 23, 2014
Merged

Fixed issue with DSP Dump not closing files. #530

merged 1 commit into from Jun 23, 2014

Conversation

workhorsy
Copy link
Contributor

When dumping the two DSP ROMs to file, if it failed to open one of them, neither will be closed. It should now close any that were opened.

When dumping the two DSP ROMs to file, if it failed to open one of them, neither will be closed. It should now close any that were opened.
@magumagu
Copy link
Contributor

FYI, you can just force-push (git push -f) onto an existing branch, and that will automatically update the PR.

@workhorsy
Copy link
Contributor Author

cool. I'll have to remember that for the future. I used push -f to re-upload the change, but I assumed that it would not connect the two because the hash would be different. Looks like git/github is smart enough to connect the two.

Thanks

@pauldacheez
Copy link
Contributor

You're choosing an entire branch to merge, not just a single commit, so you can destroy and recreate commits any way you want and it'll still recognize what you're pushing to.

Also, this is basic info, but if you ever want to have more than one PR open for a project at a time, you'll need to make separate non-master branches (git branch fix-dspspy-bugs && git checkout fix-dspspy-bugs) and push those. Merges from user:master into upstream:master just seem kinda lazy anyway, IMO.

@lioncash
Copy link
Member

@dolphin-emu-bot rebuild

Doing a build check, and then it should be good to merge.

@workhorsy
Copy link
Contributor Author

Thanks for the tips @pauldacheez . I usually do work and playing around in a separate branch. I was so concerned with not breaking my first commit to Dolphin, that I forgot to put my changes in a separate branch.

It's so nice here with all the people trying to help each other. Usually people just ignore new contributors if they do something wrong/not quite right. Thanks.

lioncash added a commit that referenced this pull request Jun 23, 2014
Fixed issue with DSP Dump not closing files.
@lioncash lioncash merged commit 4138702 into dolphin-emu:master Jun 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants