-
Notifications
You must be signed in to change notification settings - Fork 5
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
Merge upstream #4
Comments
Is v0.5.0_beta4 a pure mirror of upstream right now? I noticed it contains only the fork commit from @chrox , so i am a little bit confused.
Yes, this should be the right approach. |
From my quick tests rebasing didn't seem to want to work with pure upstream, but merging proper 0.4.2 into @chrox' initial commit was no issue (since they're identical). Plus I think that's preferable anyway for having the history available. From that point onward you can do pretty much whatever you want. Specifically, I want to |
So if I understand it correctly, the new master will just be all the history in svn + our own commits? Ideally, renaming master branch should be avoided once it becomes public. In this case, renaming is probably fine because it's only used by us :) |
Here's a sample upstream with our patches on top, for now mostly for the purpose of getting this stuff into upstream but in the hopefully nearby future in order to become our new default. https://github.com/Frenzie/sdcv/commits/v0.5.4_beta4_koreader_changes It compiles and seems to work okay. |
Looks good to me. How about the one committed by chrox (23eda3e)? Is it going to stay only in our fork? |
You mean you want it gone from the history? On May 7, 2016 12:56 AM, "Qingping Hou" notifications@github.com wrote:
|
Sorry that I missed the discussions here. It doesn't matter if we include the first huge commit or not. Even rewriting the whole commit history should be OK, because each repo of this project has only one releasing branch. We don't need to maintain multiple snapshots for different platforms, which will make life much easier. So cherry-picking the real patches on top of the upstream 0.5.0_beta should be fine. |
No problem. Could you take a look at https://github.com/Frenzie/sdcv/commits/v0.5.4_beta4_koreader_changes to see if I missed anything pertinent? Those are the patches I've already submitted upstream over at https://sourceforge.net/p/sdcv/patches/16/ Most important, I don't know if 7f21045 is automagically fixed by the upstream change to CMake or if it might need to be reapplied somehow. |
The patches look good to me. 7f21045 is vital to build sdcv on Android and I will test on your branch and see if it's already capable to build on Android. Thanks for your work. |
Done. Upstream integrated all of our patches. |
I've got upstream ready over at https://github.com/Frenzie/sdcv/tree/v0.5.0_beta4
List of our commits that seem relevant:
58ef78c add git ignore file(edit: actually, this one doesn't seem relevant now that I've actually looked at it)4c68c30 add cJSON library
17071c7 add json-output option
b952af1 Add support for the ' entity(already in upstream)3f77095 add support for XML formated dictionary data(already in upstream)9ee1c9e add suppport for HTML and Wikimedia dictionary data
53fe815 print save to cache message to stderr so that it won't mess up JSON data
0d3852a get rid of 'Nothing similar to' message when there is no dict
@chrox Do you think I'm missing anything or that there's anything superfluous?
The final step would be to replace master. I was thinking of maybe renaming it to something else and making my v0.5.0_beta4 the new master after the merge?
A related issue is how to contribute our changes back to upstream. Something like
git format-patch COMMIT
sounds like it should be about right to generate patches to submit upstream.The text was updated successfully, but these errors were encountered: