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

adopt pull-requests fom vis and visjs-network #7

Closed
14 tasks done
mojoaxel opened this issue Jul 23, 2019 · 5 comments · Fixed by #15, #16 or #19
Closed
14 tasks done

adopt pull-requests fom vis and visjs-network #7

mojoaxel opened this issue Jul 23, 2019 · 5 comments · Fixed by #15, #16 or #19
Labels
enhancement New feature or request
Milestone

Comments

@mojoaxel
Copy link
Member

mojoaxel commented Jul 23, 2019

We should think about how to adopt (old) pull-requests from vis and visjs-network.

Here you can follow the merging progress done in visjs-network: visjs-community/visjs-network#27

Also what would be a good way to provide attribution to the original contributor!?

@mojoaxel mojoaxel added the enhancement New feature or request label Jul 23, 2019
@richardeaxon
Copy link
Contributor

There are a couple of ways to do this (see here https://stackoverflow.com/questions/6022302/how-to-apply-unmerged-upstream-pull-requests-from-other-forks-into-my-fork).

Lets say I want to target almende/vis pull #4280 and merge it into the new vis-network. I could do

git pull https://github.com/almende/vis refs/pull/4280/head

which sucks the entire pull request across but generates masses of merge conflicts due to the structural changes of the new vis-network.

I can also cherry-pick which works just fine for a a single commit like in this pull request. Just wack in the commit ID in almende/vis and git/github figures things out automagically. No merge conflicts and the correct attribution and log messages come across as per the original commit.

git cherry-pick 9726da595bd1fcb4c5c7264a53fed28f02d69a55

I am not sure how to handle bigger pulls with multiple commits. Can cherry-pick do that?

@richardeaxon
Copy link
Contributor

And from here (https://stackoverflow.com/questions/1670970/how-to-cherry-pick-multiple-commits) you can cherry-pick ranges.

git cherry-pick 88b1c05abafcd0aa51e89ba417cd4cbc42a2ae60..4c604a38818c96ae252ed2cbd50143f4092a341b

I did notice that I still had to import the entire remote pull request to load all the refs before I could cherry-pick. So my procedure was:

# for a single commit in the pull
git pull https://github.com/almende/vis refs/pull/4280/head
git merge --abort
git cherry-pick 9726da595bd1fcb4c5c7264a53fed28f02d69a55 

# or for a range of commits in a pull
git pull https://github.com/almende/vis refs/pull/4205/head
git merge --abort
git cherry-pick 88b1c05abafcd0aa51e89ba417cd4cbc42a2ae60..4c604a38818c96ae252ed2cbd50143f4092a341b

The second example has merge conflicts which would need fixing.

This still requires some manual work and analysis of each pull request to see where it belongs and if it is still valid.

@mojoaxel
Copy link
Member Author

@richardeaxon Thanks! Feel free to provide some useful pull-requests 😉

@mojoaxel
Copy link
Member Author

I followed this and was successfull. Here is an example:

git remote add visjs-network https://github.com/visjs-community/visjs-network.git
git fetch visjs-network
git checkout -b fix-cluster-open-missing-edge-labels
git cherry-pick 89c8ee285a320097b52371e760d6bc41200dbae7
...

@mojoaxel
Copy link
Member Author

🎉 vis-timeline@5.1.0 is now feature-compatible with visjs-network@4.24.10 🚀

@mojoaxel mojoaxel changed the title adopt pull-requests fom vis and visjs-timeline adopt pull-requests fom vis and visjs-network Aug 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants