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

Readme.MD, Premake5, Swedish comment translation to English in code, au-package.sh quickfix, wavetable rmb quickfix, alert on mac if no configuration.xml #68

Closed
wants to merge 53 commits into from

Conversation

esaruoho
Copy link
Collaborator

@esaruoho esaruoho commented Dec 13, 2018

This PR updates

@kurasu @kzantow @abique thoughts appreciated.

heads up, @baconpaul - not sure if these help but just on the offchance they do, have fun!

Fixes #67
Fixes #29 (Thanks @baconpaul!)
Fixes #89 (Thanks @baconpaul)
Fixes #77 (Thanks @kzantow)

@esaruoho esaruoho changed the title Readme.MD update, Premake5 beautify, Swedish comment translation to English in code + au-package.sh quickfix + wavetable rmb quickfix Readme.MD, Premake5, Swedish comment translation to English in code, au-package.sh quickfix, wavetable rmb quickfix, alert on mac if no configuration.xml Dec 16, 2018
@esaruoho
Copy link
Collaborator Author

Added @baconpaul 's quickfix from #89 for mac.( link to commit baconpaul@e5737f0 )

@kzantow
Copy link
Collaborator

kzantow commented Dec 16, 2018

@esaruoho fix for #77 is kurasu@7be5243

@esaruoho
Copy link
Collaborator Author

@kzantow thanks, i'll add that to #68 too!

@baconpaul
Copy link
Collaborator

I think we should maybe just have one big pull request which gets us to zoomable ui plus all your fixes then give the maintainers one thing to review rather than these scattered cross commits I have one commitnfor the logo display but otherwise my code is done and meegable

Would someone like to take a crack at making a branch which rldoes these merges or making a super pr?

@kzantow
Copy link
Collaborator

kzantow commented Dec 16, 2018

@baconpaul I agree, it would be much easier to review and fix in one spot. I can make a PR with all the combined stuff later today

@baconpaul
Copy link
Collaborator

baconpaul commented Dec 16, 2018 via email

Add Surge.pdf documentation
@esaruoho
Copy link
Collaborator Author

PR #70 content added

@esaruoho esaruoho closed this Dec 16, 2018
@esaruoho esaruoho reopened this Dec 16, 2018
@esaruoho
Copy link
Collaborator Author

@baconpaul @kzantow i merged a couple of things into this (like the surge pdf, and #70 ..) but was unable to add zoomable-ui into the mix due to issues. i'm also worried about this:

kurasu@c4c6a08

was trying to get my fork in sync with the main repo and i guess i failed?

@baconpaul
Copy link
Collaborator

Oooh that diff is a bit odd. I would not include that.

Which pull request are we using for the "master"? Shall we just use #69? I am happy to add zoomable-ui to that now and then for the rest to get added in?

@esaruoho
Copy link
Collaborator Author

@baconpaul yeah i was doing my head in trying to cherry-pick commits from this branch into another branch, but i just don't know enough git-fu to get rid of that diff or to remove it from the branch or something. really confusing. i wish i knew how.

@baconpaul
Copy link
Collaborator

From https://stackoverflow.com/questions/36168839/how-to-remove-commits-from-a-pull-request

# Checkout the desired branch
git checkout <branch>

# Undo the desired commit
git revert <commit>

# Update the remote with the undo of the code
git push origin <branch>

where is the branch in your forked repo with all of this and is c4c6a08

but at some points in git merge hell it's easier to start a new branch and remerge and redo a PR. Which is why branching is a bit tricky when dev cadence is faster than merge cadence.

@esaruoho
Copy link
Collaborator Author

@baconpaul hmm.. i'm getting this

$ git revert c4c6a08
error: commit c4c6a08ed3f14ddab06e98d29a06c74b8174524f is a merge but no -m option was given.
fatal: revert failed

and

$ git revert -m c4c6a08
error: option `mainline' expects a number greater than zero

and

$ git revert c4c6a08ed3f14ddab06e98d29a06c74b8174524f
error: commit c4c6a08ed3f14ddab06e98d29a06c74b8174524f is a merge but no -m option was given.
fatal: revert failed

and

$ git revert -m c4c6a08ed3f14ddab06e98d29a06c74b8174524f
error: option `mainline' expects a number greater than zero

.. ouch

@baconpaul
Copy link
Collaborator

Yuck. I’m not sure then.

@esaruoho
Copy link
Collaborator Author

I'll try and figure it out. Thing is, my fork has that diff there already, since I tried to update the fork so it'd be in-tune with kurasu:surge. if i delete the fork, all the changes vanish too.

..don't feel like opening a second github account either.. surely there has to be a way?

@esaruoho
Copy link
Collaborator Author

maybe if i could figure out a way of making a new branch from say, your branch, then i could start rebuilding a new PR. i'll investigate this tomorrow. mind you, it might be trivial for @kurasu to just remove the diff or something, but.. i guess we'll be wiser come monday-wednesday.

@baconpaul
Copy link
Collaborator

@esaruoho looks like @kzantow has got most of this pulled into his #92 without this weird diff. He definitely got all of my key changes into that PR and I closed #69 as a result. So perhaps close this one too and then we can just all use his #92 for the review and merge to get zoomable documented etc... all in place.

@esaruoho
Copy link
Collaborator Author

all this everything in #92, so closing.

@esaruoho esaruoho closed this Dec 17, 2018
@esaruoho esaruoho deleted the documentation-beautify-swedish-translation branch December 18, 2018 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants