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

fixed collision contact tutorial failing build due to image location… #94

Merged
merged 3 commits into from
Aug 15, 2017

Conversation

jwhendy
Copy link
Contributor

@jwhendy jwhendy commented Aug 3, 2017

… and not being referenced by a toctree

I kept getting failures on a pretty simple PR (#93) and found it's related to #87 . This fixes (well, at least one way to do it) two issues I found:

  • image path was /ros_visualization/collision_contact_tutorial_screen.png and I think that hard path was wrong. I didn't get it to play nicely, so I moved it into the same folder as the .rst (doc/) like all of the other tutorial image files

  • after fixing that, I was getting a failure related to this tutorial not being referenced by a toctree. I found this SO post saying one could add :orphan: to the top of the file.

If this is desired in some table of contents, I'll leave that to someone else to fix. This just gave some peace of mind as I thought my PR was causing an issue...

@jwhendy
Copy link
Contributor Author

jwhendy commented Aug 3, 2017

Instead of using the :orphan: trick, it dawned on me that this might not add the tutorial anywhere. I added it to index.rst and removed :orphan: so hopefully that does the trick.

@0ldham
Copy link
Contributor

0ldham commented Aug 7, 2017

My mistake! Thanks for the PR; looks good.

@davetcoleman davetcoleman merged commit 7c1a9b8 into moveit:kinetic-devel Aug 15, 2017
@davetcoleman
Copy link
Member

@jwhendy thanks for correcting this. The errors from our CI builds are currently difficult because it has a valid URL checker that always when new tutorials or image assets are added. This is because it checks the current website for those files, when in fact the files won't exist until after the PR is merged. Because of this, I have to merge PRs even when travis fails based on visual inspection of the reasons it failed, which can be error prone. If you had motivation to improve our CI, it would be great to have it run tests on the new website (including changes) rather than the current one.

@jwhendy
Copy link
Contributor Author

jwhendy commented Aug 24, 2017

I think I follow that? Basically if someone adds some file to /path/to/file.ext, it doesn't exist yet and thus the build fails no matter what. Then you merge, the file gets created, and then you get the real feedback on the build. Is that halfway accurate?

I have zero Cl experience, but seems pretty great especially for a huge project like ROS! I may add it to my mental "someday to look into" list!

davetcoleman pushed a commit to davetcoleman/moveit_tutorials that referenced this pull request Oct 19, 2017
…moveit#94)

* fixed failing build due to image location and not being referenced by a toctree

* forgot to add image file

* added collision contact tutorial to index.rst in advanced section
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.

3 participants