Skip to content

Conversation

@JafarAbdi
Copy link
Member

Description

Please explain the changes you made, including a reference to the related issue if applicable

Checklist

  • Tested modified webpage locally using the build_locally.sh script
  • While waiting for someone to review your request, please consider reviewing another open pull request to support the maintainers

@mamoll
Copy link
Contributor

mamoll commented Sep 22, 2020

Looks good. We just need to remember to update the link to the tutorial to the official moveit_tutorials repo once moveit/moveit_tutorials#521 is merged. I just approved that PR and asked John to review it as well, so maybe just give it another day, in case we can that PR merged first.

@mamoll mamoll requested a review from JStech September 28, 2020 16:03
@JafarAbdi JafarAbdi force-pushed the pr-deep_grasping_post branch from 1f292bc to bb115b6 Compare September 28, 2020 16:27
@JStech
Copy link
Contributor

JStech commented Sep 28, 2020

Viewing the post through GitHub is not rendering the table with the two images correctly (it's showing the markup in-line). I don't know, though, if that needs to be fixed here or if it will still render correctly when viewed through the website.

@mamoll
Copy link
Contributor

mamoll commented Sep 28, 2020

It renders correctly when built locally. @JStech if that was the only issue, can you approve and merge?

@JStech
Copy link
Contributor

JStech commented Sep 29, 2020

Approved. I can't merge, though.

Copy link
Contributor

@felixvd felixvd left a comment

Choose a reason for hiding this comment

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

Didn't build it locally, but it looks good to me. I'm good with merging after the comment is addressed (I don't insist on that change in particular, but just have another look at it).

@JafarAbdi JafarAbdi merged commit d14e705 into moveit:main Sep 29, 2020
@JafarAbdi JafarAbdi deleted the pr-deep_grasping_post branch September 29, 2020 10:05
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.

4 participants