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

Externals: Add Dear Implot #11356

Merged
merged 2 commits into from Dec 23, 2022
Merged

Conversation

Sam-Belliveau
Copy link
Contributor

@Sam-Belliveau Sam-Belliveau commented Dec 20, 2022

This is a dependency for the branches I am working on. I am currently blocked on this.

I do not know if this builds on windows.

@iwubcode
Copy link
Contributor

This should be using a submodule for the implot code. See my SPIRVCross implementation for an example.

@@ -168,6 +168,7 @@ PRIVATE
spng
xxhash
imgui
implot
Copy link
Contributor

Choose a reason for hiding this comment

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

Changes to add implot as a dependency should be done in the PR that is actually using them

@Sam-Belliveau
Copy link
Contributor Author

This should be using a submodule for the implot code. See my SPIRVCross implementation for an example.

is there a reason that ImGui is not done like this? I just wanted to keep it consistent with ImGui.

@iwubcode
Copy link
Contributor

is there a reason that ImGui is not done like this? I just wanted to keep it consistent with ImGui.

@Sam-Belliveau - it is an older dependency

Copy link
Contributor

@iwubcode iwubcode left a comment

Choose a reason for hiding this comment

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

Looks ok to me. Thanks!

@BhaaLseN
Copy link
Member

Looks good in general, but you might wanna clean up those commits (because many of them have no discernable meaning when looked at in isolation).


To squash those commits into one before continuing; you can use the following commands (replace upstream with the name of the Dolphin remote and origin with the name of your Fork remote. Also, make sure you are on your branch when you do this):

git fetch upstream
git reset --soft upstream/master
git commit -m "your commit message here"
git push -f origin HEAD

The first command makes sure you get the latest commits from master (from the Dolphin repository) to work on; the second picks all your changes, switches to that master state and puts the changes in the staging area. The third command then commits it so you have one commit total that has all the changes (which you can also replace with your commit method of choice, such as git gui or any other GUI tool); and the forth command pushes your current branch into your own remote (which should be your Fork) while overwriting all previous changes (to update this PR).

@Sam-Belliveau
Copy link
Contributor Author

Looks good in general, but you might wanna clean up those commits (because many of them have no discernable meaning when looked at in isolation).

To squash those commits into one before continuing; you can use the following commands (replace upstream with the name of the Dolphin remote and origin with the name of your Fork remote. Also, make sure you are on your branch when you do this):

git fetch upstream
git reset --soft upstream/master
git commit -m "your commit message here"
git push -f origin HEAD

The first command makes sure you get the latest commits from master (from the Dolphin repository) to work on; the second picks all your changes, switches to that master state and puts the changes in the staging area. The third command then commits it so you have one commit total that has all the changes (which you can also replace with your commit method of choice, such as git gui or any other GUI tool); and the forth command pushes your current branch into your own remote (which should be your Fork) while overwriting all previous changes (to update this PR).

Thank you I really could not remember how to do this

@delroth delroth merged commit 2345ba1 into dolphin-emu:master Dec 23, 2022
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants