Skip to content

Melodic-devel sync#35

Closed
henningkayser wants to merge 28 commits intomoveit:masterfrom
henningkayser:master
Closed

Melodic-devel sync#35
henningkayser wants to merge 28 commits intomoveit:masterfrom
henningkayser:master

Conversation

@henningkayser
Copy link
Copy Markdown
Member

There are some more commits to sync this time.
@davetcoleman I followed your instructions from here and used a merge commit for syncing my master-branch. There were some simple conflicts so that Github added two merge commits. Should I squash those?

Alternatively we could use rebase --onto so we don't have extra merge commits but potentially altered history.

130s and others added 28 commits February 23, 2019 12:17
[melodic] Update version info to 1.0.0
* clang-tidy: performance-faster-string-find

* clang-tidy: performance-for-range-copy

* clang-tidy: performance-unnecessary-copy-initialization

* clang-tidy: clang-tidy: performance-unnecessary-value-param

* clang-tidy: modernize-redundant-void-arg

* clang-tidy: modernize-use-nullptr

* clang-tidy: modernize-use-default

* clang-tidy: readability-named-parameter

* clang-tidy: readability-redundant-string-cstr

* clang-tidy: readability-simplify-boolean-expr

* clang-tidy: readability-simplify-boolean-expr (manual fix)

* clang-tidy: readability-container-size-empty

* clang-tidy: llvm-namespace-comment

* clang-format

* Keep default values setting for QFlags variable

* Further simplify boolean expression
... after fkie/catkin_lint#62 has been resolved
and catkin_lint 1.5.6 has been released.
* Switch to master branch

* Clarify release branch policy
If the main planner fails, res.trajectory_ is NULL, and accessing it segfaults.
Kinematics unit tests use random sampling, which might (randomly) cause the tests to fail (unfortunately sampling many failing samples).
This fix reduces randomness by using a fixed seed to sample robot states.

Additionally, the "searchIKWithCallback" test should resample dismissed samples as pointed out
in moveit/moveit_kinematics_tests#16.
@mlautman
Copy link
Copy Markdown
Contributor

mlautman commented Mar 8, 2019

I reviewed the changes and they don't seem to modify any of the ROS2 progress so +1 there.

rebase --onto seems better to me but @rhaschke pushed for merge commits. Personally, I prefer cherry-picking or rebasing over merging whenever possible as merge-commits dirty up the git history quite a bit. So long as the cherry-picking is going from MoveIt1 -> MoveIt2 I don't see why that would change the commit history..

@davetcoleman
Copy link
Copy Markdown
Member

We have GitHub Merge Policies and I don't claim to be an expert but basically @v4hn says merging is fine sometimes.

potentially altered history.

Will this break the synchronization with acutronic/moveit2?

@rhaschke
Copy link
Copy Markdown
Contributor

rhaschke commented Mar 8, 2019

Definitely, please use merge commits! Using cherry-picks, it becomes terribly hard to really keep track of all changes. With merge commits, you can immediately recognize which commits are missing.

There were some simple conflicts so that Github added two merge commits.

@henningkayser, I don't get why you have two merge commits here (in both directions). Did you use github to resolve conflicts? I can try later to do a cleaner merge.

@rhaschke rhaschke mentioned this pull request Mar 8, 2019
@rhaschke
Copy link
Copy Markdown
Contributor

rhaschke commented Mar 8, 2019

Closing in favor of #36.

@henningkayser
Copy link
Copy Markdown
Member Author

Definitely, please use merge commits! Using cherry-picks, it becomes terribly hard to really keep track of all changes. With merge commits, you can immediately recognize which commits are missing.

There were some simple conflicts so that Github added two merge commits.

@henningkayser, I don't get why you have two merge commits here (in both directions). Did you use github to resolve conflicts? I can try later to do a cleaner merge.

Yes I used Github to resolve conflicts which added the first commit and then merged the PR which added the second one.
I'm really not a friend of Github's web interface since it sometimes does strange things.
@rhaschke Thanks for cleaning this up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants