Conversation
| addBackgroundJob(boost::bind(&MotionPlanningDisplay::publishInteractiveMarkers, this, false), "publishInteractiveMark" | ||
| "ers"); | ||
| addBackgroundJob(boost::bind(&MotionPlanningDisplay::publishInteractiveMarkers, this, false), | ||
| "publishInteractiveMarkers"); |
There was a problem hiding this comment.
Splitting literal strings like this was awful.
|
+1 for this change. |
2293649 to
49d2ed0
Compare
|
Rebased / reapplied settings to resolve conflicts. |
davetcoleman
left a comment
There was a problem hiding this comment.
I'm not convinced this minor tweak is worth the code disruption of other PRs... some of the changes I like, but most are not any better and some are worse. I'm fine with being outvoted on this, however.
| active_joint_model_vector_[i]->getVariableRandomPositionsNearBy(rng, values + active_joint_model_start_index_[i], | ||
| *active_joint_bounds[i], | ||
| near + active_joint_model_start_index_[i], | ||
| distance); |
There was a problem hiding this comment.
i prefer the old version - this change is less readable IMO and adds another line.
| active_joint_model_vector_[i]->getVariableRandomPositionsNearBy(rng, values + active_joint_model_start_index_[i], | ||
| *active_joint_bounds[i], | ||
| near + active_joint_model_start_index_[i], | ||
| distances[i]); |
|
I agree that this PR should only be merged after other large pending PRs. |
49d2ed0 to
b8c27bc
Compare
|
@davetcoleman Could you please have another look. I further tweaked the clang-format settings, such that the source lines you complained about are not touched anymore. |
davetcoleman
left a comment
There was a problem hiding this comment.
It does look better now!
This is an attempt to improve the results of clang-format. Particularly:
increase penalty to break before first argument in function callsI think the resulting code is much better readable.