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

Added README.md for dummy_map_server #572

Merged
merged 7 commits into from
Aug 18, 2022

Conversation

cardboardcode
Copy link
Contributor

Purpose of Pull Request

This pull request adds a README.md to dummy_map_server as part of efforts to address the issue highlighted in #531.

@audrow audrow self-assigned this Jul 26, 2022
@audrow audrow self-requested a review July 26, 2022 17:01
Copy link
Member

@audrow audrow left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @cardboardcode! I left a few comments. The most major thing is that the commandline blocks be modified so the information is also correct on Windows. Perhaps that means removing most of the commands and just focusing on the commands that are the same, like the ros2 and colcon commands.

dummy_robot/dummy_map_server/README.md Outdated Show resolved Hide resolved
dummy_robot/dummy_map_server/README.md Outdated Show resolved Hide resolved
dummy_robot/dummy_map_server/README.md Show resolved Hide resolved
dummy_robot/dummy_map_server/README.md Outdated Show resolved Hide resolved
@audrow
Copy link
Member

audrow commented Jul 27, 2022

@cardboardcode, let's get these READMEs in one at a time, since you'll learn from the feedback and can apply the feedback to the other PRs, instead of me having to leave the same comment on multiple PRs.

@cardboardcode cardboardcode force-pushed the add_dummy_map_server_readme branch 2 times, most recently from 076cf9d to 4d86cb1 Compare July 28, 2022 03:49
@cardboardcode
Copy link
Contributor Author

Hi @audrow, the requested changes have been included in the latest commit for this pull request.

Will need your help to review once more. Thank you.

Copy link
Member

@audrow audrow left a comment

Choose a reason for hiding this comment

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

One nitpick and then this looks good to me!

dummy_robot/dummy_map_server/README.md Outdated Show resolved Hide resolved
@cardboardcode cardboardcode force-pushed the add_dummy_map_server_readme branch from f068207 to a290e4d Compare August 18, 2022 02:20
@cardboardcode
Copy link
Contributor Author

One nitpick and then this looks good to me!

@audrow, thank you for reviewing the changes.

Will attempt to apply the same feedback as much as I can with other similar Add README.md Pull Requests.

@audrow
Copy link
Member

audrow commented Aug 18, 2022

Will attempt to apply the same feedback as much as I can with other similar Add README.md Pull Requests.

Great, you can @ me when they're ready for review!

@audrow
Copy link
Member

audrow commented Aug 18, 2022

@cardboardcode, One small thing, if you can, avoid force pushing to your branch. It makes it so that I can't see what has changed since my last review. I understand that you may need to force push if there are conflicts.

@audrow audrow merged commit dcf8814 into ros2:rolling Aug 18, 2022
cardboardcode added a commit to cardboardcode/demos that referenced this pull request Jan 23, 2023
* ➕ 📖 Added README.md for dummy_map_server ROS2 package. Verified instructions.

Signed-off-by: Bey Hao Yun <[email protected]>

* Update dummy_robot/dummy_map_server/README.md

Decomposed sentence into smaller sizes.

Co-authored-by: Audrow Nash <[email protected]>
Signed-off-by: Bey Hao Yun <[email protected]>

* Update dummy_robot/dummy_map_server/README.md

Reworded.

Co-authored-by: Audrow Nash <[email protected]>
Signed-off-by: Bey Hao Yun <[email protected]>

* 🔨 Modified use of --packages-select to --packages-up-to based on feedback.

Signed-off-by: Bey Hao Yun <[email protected]>

* 🔨 Modified README.md further to adhere to feedback given.

Signed-off-by: Bey Hao Yun <[email protected]>

* 🔨 Modified README.md commands to adhere to higher cross-platform compatibility.

Signed-off-by: Bey Hao Yun <[email protected]>

* Update dummy_robot/dummy_map_server/README.md

Co-authored-by: Audrow Nash <[email protected]>
Signed-off-by: Bey Hao Yun <[email protected]>

Signed-off-by: Bey Hao Yun <[email protected]>
Co-authored-by: Audrow Nash <[email protected]>
cardboardcode added a commit to cardboardcode/demos that referenced this pull request Jan 23, 2023
* ➕ 📖 Added README.md for dummy_map_server ROS2 package. Verified instructions.

Signed-off-by: Bey Hao Yun <[email protected]>

* Update dummy_robot/dummy_map_server/README.md

Decomposed sentence into smaller sizes.

Co-authored-by: Audrow Nash <[email protected]>
Signed-off-by: Bey Hao Yun <[email protected]>

* Update dummy_robot/dummy_map_server/README.md

Reworded.

Co-authored-by: Audrow Nash <[email protected]>
Signed-off-by: Bey Hao Yun <[email protected]>

* 🔨 Modified use of --packages-select to --packages-up-to based on feedback.

Signed-off-by: Bey Hao Yun <[email protected]>

* 🔨 Modified README.md further to adhere to feedback given.

Signed-off-by: Bey Hao Yun <[email protected]>

* 🔨 Modified README.md commands to adhere to higher cross-platform compatibility.

Signed-off-by: Bey Hao Yun <[email protected]>

* Update dummy_robot/dummy_map_server/README.md

Co-authored-by: Audrow Nash <[email protected]>
Signed-off-by: Bey Hao Yun <[email protected]>

Signed-off-by: Bey Hao Yun <[email protected]>
Co-authored-by: Audrow Nash <[email protected]>
Signed-off-by: Bey Hao Yun <[email protected]>
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