Skip to content
This repository has been archived by the owner on Nov 13, 2017. It is now read-only.

cherry-picked #713 from indigo-devel #718

Merged
merged 1 commit into from
Jul 18, 2016
Merged

Conversation

rhaschke
Copy link
Contributor

  • cleanup structure of updateQueryStateHelper
  • updateStartStateToCurrent after execution
  • visualization: stop execution button
  • suppress warning "Execution of motions should always start at the robot's current state." when planning+executing
  • rviz display: disable Query Start State by default

- cleanup structure of updateQueryStateHelper
- updateStartStateToCurrent after execution
- visualization: stop execution button
- suppress warning "Execution of motions should always start at the robot's current state." when planning+executing
- rviz display: disable Query Start State by default
@davetcoleman
Copy link
Member

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

@davetcoleman davetcoleman merged commit 034800d into jade-devel Jul 18, 2016
@davetcoleman davetcoleman deleted the wip-stop-execution branch July 18, 2016 09:39
@rhaschke
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

v4hn commented Jul 18, 2016

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
Copy link
Contributor

v4hn commented Jul 18, 2016

...

@rhaschke
Copy link
Contributor Author

rhaschke commented Jul 18, 2016

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
Copy link
Contributor

v4hn commented Jul 18, 2016

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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.

@v4hn
Copy link
Contributor

v4hn commented Jul 19, 2016

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. :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants