-
Notifications
You must be signed in to change notification settings - Fork 16
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
Incompatible forks | Clean Git History #11
Comments
Sorry for a late reply, this issue somehow flew under my radar completely. Let me talk about my motivations for the fork first so you can figure out what would suit you best. The main changes done in this fork is largely to
The things that break the tensorflow plugin are mainly the 3, 4, and 5 changes. If you can replace all the differing dependencies found in https://github.com/getnamo/tensorflow-ue4/blob/master/Content/Scripts/TensorFlowComponent.py you should be able to setup your own fork/change to work with the rest of the plugin. Specifically it's Checking your link 20tab/UnrealEnginePython@master...getnamo:master, it appears the two are currently compatible. I generally just periodically sync with the master, but I'm not convinced all my changes make sense for the upstream fork. It would require some splitting to figure out what minimal changes would make sense to merge upstream without poluting it with opinionated changes such as embedding or plugin script isolation. If the upstream comes up with better suited solutions to 1-5, I'll gladly replace the workaround ones used here. I'm open to suggestions, but I'll likely default to the simpler path for the moment due to time limitations. |
Hi Jan,
I was using your https://github.com/getnamo/tensorflow-ue4 inside my project.
Which required
https://github.com/getnamo/UnrealEnginePython/
instead of
https://github.com/20tab/UnrealEnginePython/
Now I already had my own fork, but since the changes were small I just did a hard reset to your version and redid the changes.
After some time I upgraded my Unreal Engine. Of course this did not work with your specific fork. So I disabled the tensorflow plugin and switched back to the original '20tab/UnrealEnginePython/'. The main reason for switching back was that Robert (20tab) pulled my changes and my tensorflow code was not yet fully implemented.
Now I would like to use Tensorflow, but it seems your fork is not compatible with the original master, check:
20tab/UnrealEnginePython@master...getnamo:master
Side Note: To avoid future headache (and typing commands) I automated updating my fork so that it is exactly same as 20tab, by using a cron command with:
git pull 20tab master --rebase
git push
(20tab is my remote name, use 'remove -v' to check your remote name)
Now I saw your comment here about creating a clean pull request:
20tab#539 (comment)
And so I wondered if you would be willing to cleanup your fork as well ^^.
So in that case we can use merge / fast forward again. (and perhaps create a pull on 20tab fork with all your changes, so that all forks stay in sync)
Looking at the compare
20tab/UnrealEnginePython@master...getnamo:master
It seems that sometimes you pull / merge within GitHub.com (e.g it shows 'verified'), while other times you did it using a git cmd.
Personally I stopped using GitHub.com pull / merge, since it contaminated the history with 'Merge branch 'master' of into '.
Now I'm not a git guru by any means, but I think it should be quite easy to fix.
Or something specified here:
http://sethrobertson.github.io/GitFixUm/fixup.html
Regard,
TQwan
Note: Sorry for the long post.
The text was updated successfully, but these errors were encountered: