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

Pull Request for ROS2-Jazzy Dockerfile #470

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

mdirzpr
Copy link

@mdirzpr mdirzpr commented Aug 23, 2024

Dear Allison,

I hope you are doing well.

I wanted to inform you that, based on the work you've done, I have created a ROS2-Jazzy Dockerfile. I have also submitted a pull request to merge it into the main repository. I would appreciate it if you could review the request and approve it if everything looks good.

Please let me know if you have any questions or need further adjustments.

Best regards,
Mahdi

@@ -1,4 +1,5 @@
{
"files.eol": "\n",
Copy link
Owner

Choose a reason for hiding this comment

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

What does this do?

Copy link
Author

Choose a reason for hiding this comment

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

I encountered a problem with line endings on Linux and Windows. Unix-based systems, like Linux and macOS, use the LF (Line Feed) character for line breaks by default. However, Windows is different and uses both CR (Carriage Return) and LF (Line Feed) characters for line breaks.

To solve this, I added a line of code to ensure that the line endings are always set to LF. This approach works on both Linux and Windows without any issues.

Copy link
Owner

Choose a reason for hiding this comment

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

Why were these files deleted?

Copy link
Author

Choose a reason for hiding this comment

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

I only need the Dockerfiles related to ROS2, so I deleted ros. If it's necessary to add them back later, I'll do so.

Copy link
Owner

Choose a reason for hiding this comment

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

Did https://github.com/athackst/dockerfiles/blob/main/ros2/jazzy.Dockerfile not work for you?

I added Jazzy support in #461

Copy link
Author

Choose a reason for hiding this comment

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

There were some small changes needed for certain commands, as the previous ones were not working. I've updated them accordingly.

Copy link
Owner

Choose a reason for hiding this comment

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

What exactly wasn't working? What did you change?

Copy link
Author

Choose a reason for hiding this comment

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

In 'Set up auto-completion for user,' line 104, I believe the command has changed.

Copy link
Owner

Choose a reason for hiding this comment

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

What exactly wasn't working? What did you change?

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