-
Notifications
You must be signed in to change notification settings - Fork 699
Remove unnecessary comment #620
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
Conversation
on googling 'how to make python file executable', didn't get the expected result. For beginners, it can be time-saving
|
Thanks for helping in improving MoveIt and open source robotics! |
felixvd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This kind of thing may seem dumb, but that's exactly why it's frustrating as a beginner.
doc/move_group_python_interface/move_group_python_interface_tutorial.rst
Outdated
Show resolved
Hide resolved
|
|
||
| Now run the Python code directly in the other shell using ``rosrun``. | ||
| Note in some instances you may need to make the python script executable: :: | ||
| Note that you may need to make the Python script executable (using ``chmod +x /path_to_file/file_name.py``): :: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The repository already tracks execution permissions and I can't find a python file without execution permissions in the repository. So I think this is the wrong place for this remark altogether.
But if we keep it nonetheless, I'm happy to add the additional explanation.
I'll deliberately not vote here and leave the decision to the next reviewer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's a fair argument too, and I'm good with either of those outcomes.
@Praveendwivedi did you have trouble executing the file or did the comment send you on this Google search for no reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @v4hn & @felixvd for the reviews. I didn't try the tutorial myself, I was just going through the tutorial and came across that comment. Since previously I've also faced such issues, I thought it would be better to include the command too.
As @v4hn clearly pointed out since the execution permissions are already tracked and no such issue is found, the whole comment seems to be redundant to me. What do you think @felixvd ?
I'm not sure if such issue may arise in the future though
|
I'm not sure if such issue may arise in the future though
If they do, I would consider that a bug in the tutorials and nothing we want to teach here. :)
Could you modify your PR to remove the comment altogether?
|
|
Congrats on getting your first MoveIt pull request merged and improving open source robotics! |
* Remove unnecessary comment on googling 'how to make python file executable', didn't get the expected result. For beginners, it can be time-saving
On googling 'how to make python file executable', didn't get the expected result. For beginners, it can be time-saving
Description
Added an example command which tells the reader how to make python file executable
Checklist