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

Initialize variable #76

Merged
merged 4 commits into from
Feb 15, 2022
Merged

Initialize variable #76

merged 4 commits into from
Feb 15, 2022

Conversation

clydemcqueen
Copy link

I noticed a few intermittent failures while doing some more testing on Rolling and Galactic. I traced it down to an uninitialized variable. I should have caught this earlier. :(

I also added a simple Dockerfile.

Rolling CI is failing, but this is related to ros-tooling/action-ros-ci#722

@wep21 @jbohren can you take a look?

@jbohren
Copy link
Member

jbohren commented Feb 15, 2022

I should have caught this earlier. :(

Sure, but such issues are expected when there's no real review done. Thanks for following up with this, the change makes sense based on how that flag is being used.

@jbohren jbohren merged commit 75b0559 into ros-drivers:ros2 Feb 15, 2022

WORKDIR /work/gscam_ws/src

RUN git clone https://github.com/ros-drivers/gscam.git -b ros2
Copy link
Collaborator

Choose a reason for hiding this comment

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

@clydemcqueen [nits] How about using COPY instead of cloning?

Copy link
Author

Choose a reason for hiding this comment

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

Interesting idea. I will work on this a bit and propose a small PR.

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.

None yet

3 participants