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

Cleanup of README.md #342

Merged
merged 15 commits into from
Mar 24, 2022
Merged

Cleanup of README.md #342

merged 15 commits into from
Mar 24, 2022

Conversation

ckaran
Copy link
Contributor

@ckaran ckaran commented Feb 3, 2022

I noticed that the README.md file had a number of minor issues that meant that it wasn't as clear as it could be. This PR clears them up.

Issues

  • I rewrapped all lines to 80 columns. This makes it a little easier to read in a text editor.
  • I added bash as the info string of every fenced code block. This makes Github render the blocks correctly, and makes copying simpler.
  • I added two new environment variables to the prerequisites. These are ROS1_INSTALL_PATH, and ROS2_INSTALL_PATH. They exist to make it simple to run the scripts regardless of which versions of ROS 1 and 2 are currently in use. Instructions on how to set them are given within the README.md

@gbiggs
Copy link
Member

gbiggs commented Feb 3, 2022

Text (and similar, e.g. Markdown) files in ROS 2 are intentionally not wrapped. We use one-sentence-per-line because it makes diffs in PRs much easier to handle.

@ckaran
Copy link
Contributor Author

ckaran commented Feb 4, 2022

Understood. I'll unwrap everything and resubmit.

@ckaran
Copy link
Contributor Author

ckaran commented Feb 4, 2022

@gbiggs I've fixed the wrapping as requested.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@ckaran
Copy link
Contributor Author

ckaran commented Feb 8, 2022

@gbiggs Thanks for suggestions! I had to do a force push to signoff the commits and get them past the DCO bot, I apologize if you need to do something on your end to resync with my commits.

@ckaran ckaran requested a review from gbiggs February 14, 2022 16:11
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
ckaran and others added 15 commits March 24, 2022 11:01
This is an initial cleanup attempt of the README.md file.  All
non-code lines have been wrapped to an 80 column width, and some
markdown was cleaned up.  Further work is needed.

Signed-off-by: Cem Karan <cem.f.karan.civ@army.mil>
Numerous fenced code blocks did not have a
[info string](https://github.github.com/gfm/#info-string), which
meant that the enclosed code block wouldn't be rendered correctly.
As it turned out, all of these were intended to be run in a bash
shell, so I added that as their info strings.

Signed-off-by: Cem Karan <cem.f.karan.civ@army.mil>
…iables.

The scripts weren't directly executable before, requiring manual
substitution of various 'variables'.  This was unfortunate as Github
makes copying and pasting code blocks simple and straightforward.

The changes in this commit make it possible to copy/paste each example
into it's own shell, which should make it slightly easier for new users
to start using the bridge.

Signed-off-by: Cem Karan <cem.f.karan.civ@army.mil>
@gbiggs commented at #342 (comment)
that files in ROS2 are intentionally not wrapped to make handling
diffs in PRs easier.  I've copied in the relevant original lines
to unwrap the lines I wrapped earlier.

Signed-off-by: Cem Karan <cem.f.karan.civ@army.mil>
Accepted suggestion from @gbiggs

Co-authored-by: Geoffrey Biggs <gbiggs@killbots.net>
Signed-off-by: Cem Karan <cem.f.karan.civ@army.mil>
Co-authored-by: Geoffrey Biggs <gbiggs@killbots.net>
Signed-off-by: Cem Karan <cem.f.karan.civ@army.mil>
Co-authored-by: Geoffrey Biggs <gbiggs@killbots.net>
Signed-off-by: Cem Karan <cem.f.karan.civ@army.mil>
Co-authored-by: Geoffrey Biggs <gbiggs@killbots.net>
Signed-off-by: Cem Karan <cem.f.karan.civ@army.mil>
Co-authored-by: Geoffrey Biggs <gbiggs@killbots.net>
Signed-off-by: Cem Karan <cem.f.karan.civ@army.mil>
Co-authored-by: Geoffrey Biggs <gbiggs@killbots.net>
Signed-off-by: Cem Karan <cem.f.karan.civ@army.mil>
Co-authored-by: Geoffrey Biggs <gbiggs@killbots.net>
Signed-off-by: Cem Karan <cem.f.karan.civ@army.mil>
Co-authored-by: Geoffrey Biggs <gbiggs@killbots.net>
Signed-off-by: Cem Karan <cem.f.karan.civ@army.mil>
Co-authored-by: Geoffrey Biggs <gbiggs@killbots.net>
Signed-off-by: Cem Karan <cem.f.karan.civ@army.mil>
…details (#346)

* The service name might provide more debug information than host:port details

Signed-off-by: Tim Clephas <tim.clephas@nobleo.nl>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Cem Karan <cem.f.karan.civ@army.mil>
Changes due to suggestions from @gbiggs in the discussion of [PR 342](#342 (comment))

Signed-off-by: Cem Karan <cem.f.karan.civ@army.mil>
@ckaran
Copy link
Contributor Author

ckaran commented Mar 24, 2022

OK, I modified 'galactic' -> 'rolling'.

@gbiggs gbiggs merged commit 852f2c4 into ros2:master Mar 24, 2022
@gbiggs
Copy link
Member

gbiggs commented Mar 24, 2022

Thanks for the contribution!

@ckaran
Copy link
Contributor Author

ckaran commented Mar 25, 2022

You're welcome!

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.

3 participants