Skip to content

Conversation

@crthilakraj
Copy link
Contributor


Basic Info

Info Please fill out this column
Ticket(s) this addresses #1608
Primary OS tested on Ubuntu
Robotic platform tested on ( gazebo simulation of Turtlebot)
@SteveMacenski SteveMacenski changed the base branch from eloquent-devel to master March 24, 2020 18:41
@SteveMacenski SteveMacenski changed the base branch from master to eloquent-devel March 24, 2020 18:41
Copy link
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to run the linters ament_uncrustify and ament_cpplint as you have unlinted code. Also, this needs to be retargeted to master.

geometry_msgs::msg::PoseStamped goal_copy;
goal_copy.pose = goal;
plan.poses.push_back(goal_copy);
if (plan.poses.size() > 2){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (plan.poses.size() > 2){
if (plan.poses.size() > 2) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected the changes requested. Regarding retarget to master, does that mean the eloquent- devel will not have these changes? or master will be merged later to eloquent-devel

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, should I create a new pull request for the master branch with cherrypicked commits from the current pull request?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Master may be eventually synced with eloquent, but all new development goes to master. You may create a new PR or rebase.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1612 have raised the new PR, we can close this PR

@codecov
Copy link

codecov bot commented Mar 25, 2020

Codecov Report

❗ No coverage uploaded for pull request base (eloquent-devel@ea1667e). Click here to learn what that means.
The diff coverage is 54.54%.

Impacted file tree graph

@@                Coverage Diff                @@
##             eloquent-devel    #1609   +/-   ##
=================================================
  Coverage                  ?   36.14%           
=================================================
  Files                     ?      233           
  Lines                     ?    12089           
  Branches                  ?     5251           
=================================================
  Hits                      ?     4369           
  Misses                    ?     4502           
  Partials                  ?     3218
Flag Coverage Δ
#project 36.14% <54.54%> (?)
Impacted Files Coverage Δ
nav2_navfn_planner/src/navfn_planner.cpp 47.02% <54.54%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea1667e...8949d9d. Read the comment docs.

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

Labels

None yet

2 participants