search

moveit / moveit_ros

THIS REPO HAS MOVED TO https://github.com/ros-planning/moveit
70 stars 118 forks source link

cherry-picked #713 from indigo-devel #718

Closed rhaschke closed 8 years ago

rhaschke commented 8 years ago
davetcoleman commented 8 years ago

... we now have a kinetic version too because of C++11

rhaschke commented 8 years ago

I would have loved to use a squashing commit, i.e. forwarding jade-devel to the cherry-pick branch. @davetcoleman @v4hn Anybody minds to rebase jade-devel to there?

Hopefully this facilitates reading the git graph: Merge commits are unique contributions, while cherry-picks could be committed directly. I just had to use a PR to let Travis do the compile checking.

rhaschke commented 8 years ago

As nobody complained, I have reset jade-devel to 3f16bda. Please be careful to continue on the correct commit, when you already pulled the jade-devel update:

git checkout jade-devel git reset --hard origin/jade-devel

v4hn commented 8 years ago

Anybody minds to rebase jade-devel to there?

Yes, please never rebase any of the official branches (unless someone decides to push 100MB+). If you don't want to see a request merged by somebody else, please indicate that in the title/description. I agree though that dave should have squash-merged though. Sounds like a nice thing to discuss next week :) I like the policy to merge once and squash/commit to other branches. Sadly, this doesn't play nice with git cherry though, so I'm unsure whether it's a good idea. In this regard rebasing the request (with all its individual commits) on top of the other branches and fast-forwarding the branches seems to be the better choice...

v4hn commented 8 years ago

...

rhaschke commented 8 years ago

Sorry, just did it. As long as nobody continued to work on that branch, there is no harm. That's why, I didn't want wait any longer. I've seen both of you being active and you didn't reply (in time) to my two requests.

v4hn commented 8 years ago

I saw that much.. Please don't next time. People (at least me) tend to freak out when wstool update / git pull fails because someone decided to rebase official branches.

davetcoleman commented 8 years ago

Yea, I agree we shouldn't ever force-push the main ros-planning repos, which is what I'm assuming you just did. I'm also not clear why you didn't cherry-pick the necessary commits into a new branch for the target ROS-version and create a PR based on those cherry-picks - isn't that what you did? Similar to how I did it here. Please don't create PRs claiming to do something I desire (cherry-pick to newer branch) but isn't labeled "do not merge".

Though I'm a bit confused by what you guys mean by squash-merged.

rhaschke commented 8 years ago

I'm also not clear why you didn't cherry-pick the necessary commits into a new branch for the target ROS-version and create a PR based on those cherry-picks - isn't that what you did?

I didn't cherry-picked all commits individually, but rebased and squashed: git rebase -i --onto <new branch> <base commit> <old branch>

To facilitate this task, github allows to "squash and merge". Simply click the option in the dropbox of the "Confirm merge" button.

Though I'm a bit confused by what you guys mean by squash-merged.

That's what we mean ;-)

davetcoleman commented 8 years ago

Interesting - I've never used that feature of Github!

screenshot from 2016-07-18 16 58 45

Seems like a superior option for keeping the git history clean - are there use cases where we would want to do a regular merge? Or should we just disable that option for all the git repos?

rhaschke commented 8 years ago

As said, I suggest to use

v4hn commented 8 years ago

On Mon, Jul 18, 2016 at 11:28:17PM -0700, Robert Haschke wrote:

As said, I suggest to use

  • regular merge commits whenever a genuine contribution (with several commits) is going to be merged. The merge directly visualizes this fact in the git history graph.
  • squash merges whenever there is only a single commit to be merged, or when we cherry-pick a set of commits from another branch. This indeed keeps the history clean.

Let's simply discuss this during the next meeting. :)