search

AdamsLair / duality

a 2D Game Development Framework
https://adamslair.github.io/duality
MIT License
1.41k stars 290 forks source link

Reworked coroutines implmentation #695

Closed SirePi closed 5 years ago

SirePi commented 5 years ago

This is a reworked implementation of the first, much more complex implementation (see PR #674) and takes into account all the comments to the previous version. It's been made as a separate PR to reduce the number of commits affecting the changes.

SirePi commented 5 years ago

Huuuu.. did I do the bad thing?

Barsonax commented 5 years ago

thats a awful lot of merge commits?

SirePi commented 5 years ago

In theory I just got back on par with the current status of the master branch.. don't know if it's still ok or I should just redo the PR. @ilexp?

Barsonax commented 5 years ago

Those commits from 2015 shouldnt be there. I cannot find the hashes of those commits either. What did you do O_o.

Barsonax commented 5 years ago

It seems that the release branch that you merged into your branch contains alot of merge commits (all the way to 2015) that are not present in the master branch: image

I think what went wrong here is in fact merging the release branch into your branch. I think you should redo it and instead of merging the release branch to your branch you should merge the master branch into your branch.

For a even cleaner history you might want to rebase your branch on top of the master branch. This should prevent all those merge commits from popping up (however only do this if you are the only one working with that branch!).

Anyway I think the fix is simple. Go back to commit ae1fca7129a7e062c519890ed99aa0f694fe882b (which is before you merged). Then merge master (NOT RELEASE) into your branch. If you want to go the rebase way then rebase your branch on top of the master branch.

ilexp commented 5 years ago

I think what went wrong here is in fact merging the release branch into your branch.

Yep. The release branch should only be the target of merges from master, but never the source - it's a one-way-street for continuous integration where every merge is a package release.

SirePi commented 5 years ago

Ah damn.. got the wrong one.. oh well, a new PR it is 😓