Skip to content

Script #1599

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.

Sign up for GitHub

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

Merged
SteveMacenski merged 3 commits into from
Mar 19, 2020
Merged

Script #1599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
77a5faa
Add eloquent option to build script and made it default
shivaang12 Mar 18, 2020
4b384b6
Add eloquent in the error message
shivaang12 Mar 18, 2020
d5ad5fb
Remove dashing
shivaang12 Mar 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions tools/build_all.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
# set at the time we start the last step, the ros1_bridge build.

if [ "$ROS2_DISTRO" = "" ]; then
Copy link
Copy Markdown
Member

@SteveMacenski SteveMacenski Mar 18, 2020

Choose a reason for hiding this comment

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

Please revert to 4 spaces. Its customary for shell script.

Copy link
Copy Markdown
Collaborator Author

@shivaang12 shivaang12 Mar 18, 2020

Choose a reason for hiding this comment

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

Do you find weird that after this block every indentation is 2 space indentation?

Copy link
Copy Markdown
Member

@SteveMacenski SteveMacenski Mar 18, 2020

Choose a reason for hiding this comment

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

Ah, that is strange. Lets just make it consistent, whether 4 or 2 I suppose.

export ROS2_DISTRO=dashing
export ROS2_DISTRO=eloquent
fi
if [ "$ROS2_DISTRO" != "dashing" ]; then
echo "ROS2_DISTRO variable must be set to dashing"
exit 1
if [ "$ROS2_DISTRO" != "eloquent" ]; then
Copy link
Copy Markdown
Member

@SteveMacenski SteveMacenski Mar 18, 2020

Choose a reason for hiding this comment

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

This can / should be just eloquent. The dashing version of this script in the dashing branch is only dashing. This should be merged into master and cherry picked into Eloquent with just eloquent.

echo "ROS2_DISTRO variable must be set to eloquent"
exit 1
fi

set -e
Expand Down
8 changes: 4 additions & 4 deletions tools/initial_ros_setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ ENABLE_BUILD=true
ENABLE_ROS2=true

if [ "$ROS2_DISTRO" = "" ]; then
export ROS2_DISTRO=dashing
export ROS2_DISTRO=eloquent
fi
if [ "$ROS2_DISTRO" != "dashing" ]; then
echo "ROS2_DISTRO variable must be set to dashing"
if [ "$ROS2_DISTRO" != "eloquent" ]; then
echo "ROS2_DISTRO variable must be set to eloquent"
exit 1
fi

Expand All @@ -24,7 +24,7 @@ for opt in "$@" ; do
*)
echo "Invalid option: $opt"
echo "Valid options:"
echo "--no-ros2 Uses the binary distribution of ROS2 dashing"
echo "--no-ros2 Uses the binary distribution of ROS2 eloquent"
echo "--download-only Skips the build step and only downloads the code"
exit 1
;;
Expand Down