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

Adds a new demo image to resources #33

Closed
wants to merge 1 commit into from

Conversation

henningkayser
Copy link
Member

@henningkayser henningkayser commented Mar 28, 2019

This is an updated image that can be used for the tutorial and or README

@henningkayser henningkayser requested a review from mlautman March 28, 2019 17:24
@mlautman
Copy link
Collaborator

Is this used somewhere?

@henningkayser
Copy link
Member Author

@davetcoleman suggested it for the tutorial (PickNikRobotics/moveit2_tutorials_old_fork#1 (comment)) and since the other images are stored here as well I thought we could just add it.
We could of course also add it to moveit/moveit_tutorials#302 directly.

@rhaschke
Copy link

The images in resources are used for README.md. If you want to use the image in the tutorial, you should add it to the tutorial.

@henningkayser
Copy link
Member Author

The images in resources are used for README.md. If you want to use the image in the tutorial, you should add it to the tutorial.

@rhaschke I agree. It's just that the same 'old' image used in the tutorial is also used in the README. I suggest adding a new one here and change both.

@rhaschke
Copy link

The key is that the tutorial should be self-contained. You cannot refer to images in another repo, can you?

@henningkayser
Copy link
Member Author

You can use raw file links for images so it doesn't matter in which repo the image is.

@rhaschke
Copy link

I consider it really bad practice to reference a (required) resource outside the tutorial package.
In future, nobody will remember that the image is referenced from the tutorial, but eventually is not used here anymore, so we easily are tempted to remove the image here.

@henningkayser henningkayser deleted the pr-new_demo_image branch April 10, 2019 18:55
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