Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request plugins explained #48

Merged
merged 46 commits into from
Sep 9, 2016
Merged

Request plugins explained #48

merged 46 commits into from
Sep 9, 2016

Conversation

v4hn
Copy link
Contributor

@v4hn v4hn commented Aug 26, 2016

The maintainers should read through the explanations and refine or correct them as required, before this goes to the website.
Here's the direct link: https://github.com/ros-planning/moveit.ros.org/blob/request-plugins-explained/documentation/plugins/index.markdown

@mikeferguson could you please have a look at the plugins you are/were involved in?
moveit/moveit#116 suggests that you are the best reviewer for the CollisionPlugin, but probably others too.

@davetcoleman
Copy link
Member

I just documented PlanningRequestAdapter

@mintar mintar mentioned this pull request Aug 29, 2016
@mintar
Copy link
Contributor

mintar commented Aug 29, 2016

As I mentioned in #55, this PR should be rebased onto gh-pages. The travis build will fail, which is a good thing: html-proofer caught its first real error (see the line comment I'll add in a few secs)!

@@ -34,6 +34,8 @@
<li><a href="/documentation/faqs/">FAQS</a></li>
<li role="separator" class="divider"></li>
<li><a href="/code-api/">Code API</a></li>
<li role="separator" class="divider"></li>
<li><a href="/plugins/">Plugin Interfaces</a></li>
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be /documentation/plugins/, not /plugins/.

Copy link
Contributor

@mintar mintar Aug 29, 2016

Choose a reason for hiding this comment

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

And perhaps move it up a bit. I think "FAQs" and "Code API" should be the last on that list, but that's a personal preference.

Also, the link to the plugin documentation should be added to documentation/index.markdown (to keep it consistent with the nav bar).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

feel free to file a request :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I corrected the broken link.

@v4hn v4hn force-pushed the request-plugins-explained branch from 3653129 to eecaf81 Compare August 29, 2016 10:17
@mintar
Copy link
Contributor

mintar commented Aug 29, 2016

Thanks for rebasing! Could you also have a look at my second comment?

And perhaps move it up a bit. I think "FAQs" and "Code API" should be the last on that list, but that's a personal preference.

Also, the link to the plugin documentation should be added to documentation/index.markdown (to keep it consistent with the nav bar).

@mintar
Copy link
Contributor

mintar commented Aug 29, 2016

Ok, I've submitted a PR for that: #60 .

@davetcoleman
Copy link
Member

davetcoleman commented Sep 7, 2016

I've just removed the "Documentation" page since it didn't have any real purpose (just an index) and it keeps the menu bar concise.

Nevermind... on further thought, I realize this is a bad idea because it serves as a landing page for documentation e.g. from the README.md for the moveit repo.

@davetcoleman
Copy link
Member

Since WMD is over, I think we should go ahead and merge this with its remaining TODOs in place, and hopefully someone can fill it in as they feel compelled. By having the template ready and easy to find, someone is more likely to add to it in the future.

@v4hn
Copy link
Contributor Author

v4hn commented Sep 8, 2016

Yeah, I'd like to merge this soon too.

Two things:

  • @davetcoleman could you finish the description of PlanningRequestAdapter?
    The "Interface Description" part should additionally point to the header file that specifies the C++ interface one has to implement for a new plugin of that type and "Concrete Implementation" should link to the code of one implemented plugin.
  • It would be great to have the different plugins a bit more visually separated from each other.
    Could we add (possibly thick) horizontal lines above each plugin header?

@davetcoleman davetcoleman force-pushed the request-plugins-explained branch 4 times, most recently from e3560f2 to 1a8a1e7 Compare September 8, 2016 21:04
@davetcoleman
Copy link
Member

I've added more info to PlanningRequestAdapter, added an extra thick line, and made each title easier to read. I also improved the way section links are used and reordered table of contents to match document.

This is ready to merge

@v4hn
Copy link
Contributor Author

v4hn commented Sep 9, 2016

Merging.

Thanks to everyone who contributed!

@v4hn v4hn merged commit 06cd50f into gh-pages Sep 9, 2016
@davetcoleman davetcoleman deleted the request-plugins-explained branch September 9, 2016 02:22
davetcoleman pushed a commit to davetcoleman/moveit.ros.org that referenced this pull request Aug 19, 2020
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.

5 participants