yijiangh / ikfast_pybind

Ikfast python binding modules built with a CMake-based build system
MIT License
37 stars 6 forks source link

No permission to create PR #2

Open Simon-Steinmann opened 4 years ago

Simon-Steinmann commented 4 years ago

Hi, I wanted to create a big pull request, adding solvers for:

However, you it seems that you disabled branching / upload permissions. Could you enable them? I think it would be cleaner to add these in your original repository instead of making a new one. Thank you :)

PS: There are major issues when trying to use solver that require Lapack. With lots of work I managed to get it to run on linux, but I had to make many changes to setup scripts etc. I left out the solvers requiring Lapack. Perhaps you know of a better solution or how to address it.

yijiangh commented 4 years ago

Hi @Simon-Steinmann , Thanks for your interest and all the efforts! These additions seem very useful!

Unfortunately, for security reasons, I cannot give you push permission. The common way to do PR is that you fork my repo, push it to your forked repo, and then create it PR to my original repo. In this way, we can have a proper review process before directly merging it here.

Thanks again for your interest!

Simon-Steinmann commented 4 years ago

okay will do. The way i'm used to do it, ist that people can create branches, but only the authors can merge.

Simon-Steinmann commented 4 years ago

https://github.com/Simon-Steinmann/ikfast_pybind_PR

Here you go. Compiled successfully on windows 10 with python 3.8

yijiangh commented 4 years ago

Thanks for the efforts - these new solvers look awesome!

However, instead of clone the repo, rename it, and push to your own github account, the right way to fork is to click the "fork" button here:

image

After forking, you can push your new commits to this forked repo, and then you can create a new Pull Request by clicking the "New Pull Request" button (image below).

image "

Then, this PR will appear at our original repo and I can review and merge it. You can see an example of this "fork-commit-PR-review-merge" process here (a screenshot below).

image

After the PR is merged, you can choose either to keep the forked repo (if you want to keep working on it in the future or just feeling lazy) or delete it to keep your GitHub account "cleaner".

Would you mind doing this? This will help us keep the commit history clean and let other people better appreciate your efforts since they can locate your developments easier.

Simon-Steinmann commented 3 years ago

Thank you for the explanation. I created the PR :)