Skip to content

Melodic - #2

Open
maciejzj wants to merge 5 commits into
masterfrom
melodic
Open

Melodic#2
maciejzj wants to merge 5 commits into
masterfrom
melodic

Conversation

@maciejzj

@maciejzj maciejzj commented Apr 7, 2020

Copy link
Copy Markdown
Member

No description provided.

@maciejzj maciejzj left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Minor comments.

}
],
"version": 4
} No newline at end of file

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

As GitHub suggests a newline is missing at the end of file.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I see that GitHub doesn't warn about it everywhere (here not). However generally all files on Unix likes should have an empty line at the bottom.
Git sometimes warns about this too.

Comment thread .vscode/settings.json
"python.autoComplete.extraPaths": [
"/opt/ros/melodic/lib/python2.7/dist-packages"
]
} No newline at end of file

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

As GitHub suggests a newline is missing at the end of file.

Comment thread install
echo -e "${GREEN}Gazebo installation done${NOCOLOR}"
# read -p "Do you wish to install simulator (Gazebo) [Y/n]" -n 1 -s ans && echo
# [ "${ans}" != "n" ] && install_gazebo
# echo -e "${GREEN}Gazebo installation done${NOCOLOR}"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This can be deleted.

Comment thread pkglist-ros
python-wstool
build-essential
ros-melodic-ros-control
ros-melodic-ros-controllers No newline at end of file

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Newline missing.

Comment thread scripts/ros_setup.sh
sudo apt update
sudo apt -y install --no-install-recommends `cat pkglist-ros`

echo "source /opt/ros/melodic/setup.bash" >> ~/.bashrc

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is redundant with

grep "${SOURCE_ROS}" ${HOME}/.bashrc 2> /dev/null || \

Afaik source is needed to the run commands below. The best would be to place source /opt/ros.... on this line. And leave mentioned line in other file as is.

Comment thread scripts/ros_setup.sh
sudo apt -y install --no-install-recommends `cat pkglist-ros`

echo "source /opt/ros/melodic/setup.bash" >> ~/.bashrc
source ~/.bashrc

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If what I wrote above is ok this would be not needed.

Comment thread scripts/ros_setup.sh
echo "source /opt/ros/melodic/setup.bash" >> ~/.bashrc
source ~/.bashrc

sudo apt install python-rosdep

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Shouldn't this go to the pkglist?

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.

2 participants