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

Feedback requested - Initial design documents templates #1

Merged
merged 9 commits into from
Jun 5, 2018

Conversation

mkhansenbot
Copy link
Collaborator

Initial design documentation templates and examples

@mkhansenbot mkhansenbot requested a review from mjeronimo June 4, 2018 21:37
@@ -0,0 +1,18 @@
# Requirement Title
The <navigation system> should be able to <shall> <do something>
Copy link

@mjeronimo mjeronimo Jun 5, 2018

Choose a reason for hiding this comment

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

I think these angle brackets should be escaped. Markdown allows for some inline markup, so these are not being interpreted correctly by the renderer. They don't show up correctly in Visual Studio code, at least. Searching online, it seems you can use a single backslash before the opening angle bracket: \<navigation system>, for example, instead of <navigation system>.

design/README.md Outdated
@@ -0,0 +1,15 @@
# ROS2 Navigation Design documenation

Choose a reason for hiding this comment

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

Typo: documenation -> documentation

## More details
- I want to be able to write or use my own collision avoidance algorithm without having to re-compile the entire stack from source
- Ideally I can just change out a node using a custom launch file
- This maps to the "Collision avoidance" use case

Choose a reason for hiding this comment

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

I'd suggest use case names be capitalized: "Collision avoidance" -> "Collision Avoidance"

design/README.md Outdated
@@ -0,0 +1,15 @@
# ROS2 Navigation Design documenation
This is where the ROS2 Navigation design documentation is being collected and vetted

Choose a reason for hiding this comment

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

"ROS2 Navigation" here is inconsistent with the previous README.md, which uses "ROS2 navigation"

design/README.md Outdated
@@ -0,0 +1,15 @@
# ROS2 Navigation Design documenation

Choose a reason for hiding this comment

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

Inconsistent capitalization. I'd say either "ROS2 Navigation design documentation" or "ROS2 Navigation Design Documentation" Should be consistent on titles.

- Why is this needed?
- What is the expected user interaction?
- What use case does this map to?

Copy link

@mjeronimo mjeronimo Jun 5, 2018

Choose a reason for hiding this comment

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

I suggest we add: "Are there any non-functional requirements associated with this functional requirement?"

@@ -0,0 +1,16 @@
# Use case Title

Choose a reason for hiding this comment

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

Inconsistent capitalization: "Use Case Title" or "Use case title"

@@ -0,0 +1,16 @@
# Use case Title
As a <Developer, Researcher, Technician, etc.> I want the robot to <action> so that <I, the robot, etc.> can <do something important>

Choose a reason for hiding this comment

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

As before, we should escape these angle brackets.

## More details
- Why is this needed?
- What is the expected user interaction?

Choose a reason for hiding this comment

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

I suggest we add: "Are there any non-functional requirements associated with this use case?"

Copy link

@mjeronimo mjeronimo left a comment

Choose a reason for hiding this comment

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

A couple typos, some inconsistent capitalization, and a few recommendations.

@mkhansenbot
Copy link
Collaborator Author

@mjeronimo - I made fixes & changes you suggested. Please review again.

Copy link

@mjeronimo mjeronimo left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@mkhansenbot mkhansenbot merged commit 6ba99d6 into ros-navigation:master Jun 5, 2018
@mkhansenbot
Copy link
Collaborator Author

Thanks for the review help!

@mjeronimo
Copy link

You bet! Looking forward to getting input from the ROS community.

@belem2050 belem2050 mentioned this pull request Jun 15, 2024
7 tasks
@tonynajjar tonynajjar mentioned this pull request Jun 18, 2024
7 tasks
@maksymdidukh maksymdidukh mentioned this pull request Jun 24, 2024
7 tasks
@tonynajjar tonynajjar mentioned this pull request Aug 8, 2024
7 tasks
@sandeepdutta sandeepdutta mentioned this pull request Sep 25, 2024
7 tasks
@tonynajjar tonynajjar mentioned this pull request Dec 3, 2024
7 tasks
This was referenced Dec 11, 2024
@ajtudela ajtudela mentioned this pull request Dec 19, 2024
7 tasks
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