From e134de9a28d8ea872ba0a0347f2c0ec6dcc6418a Mon Sep 17 00:00:00 2001 From: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> Date: Mon, 19 Aug 2024 19:15:55 +0900 Subject: [PATCH 01/14] docs(contributing): update the coding guideline Signed-off-by: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> --- docs/contributing/coding-guidelines/index.md | 41 ++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/docs/contributing/coding-guidelines/index.md b/docs/contributing/coding-guidelines/index.md index 5bb53e4d94b..89ea3b654fb 100644 --- a/docs/contributing/coding-guidelines/index.md +++ b/docs/contributing/coding-guidelines/index.md @@ -14,6 +14,47 @@ Also, keep in mind the following concepts. - Keep things consistent. - Automate where possible, using simple checks for formatting, syntax, etc. +- Write comments and documentation in English. +- Functions that are too complex (low cohesion) should be appropriately split into smaller functions. +- Try to minimize the use of member variables or global variables that have a large scope. +- Whenever possible, break large pull requests into smaller, manageable PRs. - When it comes to code reviews, don't spend too much time on trivial disagreements. For details see: - - + +## C++ Style Guide + +As a basic rule, follow [the Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html) and [the C++ Core Guidelines](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md). The following are common areas requiring special attention: + +- Use fixed-width integer types like `uint32_t` instead of `int`. + - cf. [How to C in 2016](https://matt.sh/howto-c) +- Use `const` for variables whose values will not change after initialization, and for functions or arguments that guarantee they will not modify the object's state. This improves code safety and clarity by preventing unintended side effects. +- When not using an index, prefer `for (const auto & e : v)`. +- For index access, prefer `.at(idx)` over `[idx]` whenever possible for safety. +- Avoid using auto with `Eigen::Matrix` or `Eigen::Vector` variables, as it can lead to bugs. +- Safe memory management is crucial for accident-free autonomous vehicles, so `new`, `delete`, `free`, and raw pointers are generally not acceptable. +- When using `mutex`, use `std::lock_guard` instead of `std::lock` and `std::unlock`. +- Use assert to validate that function arguments and return values meet specific conditions or expectations. +- Be cautious when using `throw`. C++ exceptions should only be used when it's acceptable for the process to terminate if uncaught. This applies to scenarios where Autoware has no other choice but to shut down. + +## ROS 2 + +As a basic rule, follow [the ROS 2 developer guide](https://docs.ros.org/en/humble/The-ROS2-Project/Contributing/Developer-Guide.html) and [the ROS nodes guidelines](./ros-nodes/class-design.md). The following are commonly pointed out areas that require attention: + +- Use `RCLCPP` instead of `printf` or `std::cout`. + - Reasons include the following: + - It allows for consistent log level management. For instance, with `RCLCPP_hoge`, you can simply set `--log_level` to adjust the log level uniformly across the application. + - You can standardize the format using `RCUTILS_CONSOLE_OUTPUT_FORMAT`. + - With `RCLCPP`, logs are automatically recorded to `/rosout`. These logs can be saved to a rosbag, which can then be replayed to review the log data. +- Follow [the directory structure guideline](./ros-nodes/directory-structure.md). +- If `RCLCPP_INFO` generates a large amount of logs, use `RCLCPP_INFO_THROTTLE`. +- Avoid setting default values in `declare_parameter`. Instead, specify them in a config file or launch file. + +## Autoware Style Guide + +For Autoware-specific styles, refer to the following: + +- Use the `autoware_` prefix for package names. +- Add implementations within the `autoware` namespace. +- In `CMakeLists.txt`, use `autoware_package()`. + - cf. [autoware_cmake README](https://github.com/autowarefoundation/autoware_cmake/tree/main/autoware_cmake) From fb3f9db46a98c087eaf9d9bfc345690fa46b9ec4 Mon Sep 17 00:00:00 2001 From: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> Date: Mon, 19 Aug 2024 19:35:10 +0900 Subject: [PATCH 02/14] docs(contributing): remove hoge Signed-off-by: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> --- docs/contributing/coding-guidelines/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing/coding-guidelines/index.md b/docs/contributing/coding-guidelines/index.md index 89ea3b654fb..db7234dbb28 100644 --- a/docs/contributing/coding-guidelines/index.md +++ b/docs/contributing/coding-guidelines/index.md @@ -43,7 +43,7 @@ As a basic rule, follow [the ROS 2 developer guide](https://docs.ros.org/en/humb - Use `RCLCPP` instead of `printf` or `std::cout`. - Reasons include the following: - - It allows for consistent log level management. For instance, with `RCLCPP_hoge`, you can simply set `--log_level` to adjust the log level uniformly across the application. + - It allows for consistent log level management. For instance, with `RCLCPP`, you can simply set `--log_level` to adjust the log level uniformly across the application. - You can standardize the format using `RCUTILS_CONSOLE_OUTPUT_FORMAT`. - With `RCLCPP`, logs are automatically recorded to `/rosout`. These logs can be saved to a rosbag, which can then be replayed to review the log data. - Follow [the directory structure guideline](./ros-nodes/directory-structure.md). From 9863a40c204b4a3aaa144dd59a27ef3e63b54cc8 Mon Sep 17 00:00:00 2001 From: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> Date: Mon, 19 Aug 2024 19:36:24 +0900 Subject: [PATCH 03/14] docs(contributing): update the subtitle Signed-off-by: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> --- docs/contributing/coding-guidelines/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing/coding-guidelines/index.md b/docs/contributing/coding-guidelines/index.md index db7234dbb28..cfee56190ec 100644 --- a/docs/contributing/coding-guidelines/index.md +++ b/docs/contributing/coding-guidelines/index.md @@ -37,7 +37,7 @@ As a basic rule, follow [the Google C++ Style Guide](https://google.github.io/st - Use assert to validate that function arguments and return values meet specific conditions or expectations. - Be cautious when using `throw`. C++ exceptions should only be used when it's acceptable for the process to terminate if uncaught. This applies to scenarios where Autoware has no other choice but to shut down. -## ROS 2 +## ROS 2 Style Guide As a basic rule, follow [the ROS 2 developer guide](https://docs.ros.org/en/humble/The-ROS2-Project/Contributing/Developer-Guide.html) and [the ROS nodes guidelines](./ros-nodes/class-design.md). The following are commonly pointed out areas that require attention: From 542fdc6b5bd78218787ab0ef2ab888ec7e273d30 Mon Sep 17 00:00:00 2001 From: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> Date: Mon, 19 Aug 2024 22:11:16 +0900 Subject: [PATCH 04/14] docs(contributing): Add rationale and sources to the coding guidelines Signed-off-by: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> --- docs/contributing/coding-guidelines/index.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/contributing/coding-guidelines/index.md b/docs/contributing/coding-guidelines/index.md index cfee56190ec..36c90b58471 100644 --- a/docs/contributing/coding-guidelines/index.md +++ b/docs/contributing/coding-guidelines/index.md @@ -30,12 +30,18 @@ As a basic rule, follow [the Google C++ Style Guide](https://google.github.io/st - cf. [How to C in 2016](https://matt.sh/howto-c) - Use `const` for variables whose values will not change after initialization, and for functions or arguments that guarantee they will not modify the object's state. This improves code safety and clarity by preventing unintended side effects. - When not using an index, prefer `for (const auto & e : v)`. + - cf. [C++ Core Guidelines, ES.71: Prefer a range-for-statement to a for-statement when there is a choice](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es71-prefer-a-range-for-statement-to-a-for-statement-when-there-is-a-choice) - For index access, prefer `.at(idx)` over `[idx]` whenever possible for safety. + - cf. [C++ Core Guidelines, Avoid bounds errors](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#slcon3-avoid-bounds-errors) - Avoid using auto with `Eigen::Matrix` or `Eigen::Vector` variables, as it can lead to bugs. -- Safe memory management is crucial for accident-free autonomous vehicles, so `new`, `delete`, `free`, and raw pointers are generally not acceptable. + - cf. [Eigen, C++11 and the auto keyword](https://eigen.tuxfamily.org/dox/TopicPitfalls.html) +- Safe memory management is crucial for accident-free autonomous vehicles, so `new`, `delete`, `malloc`, `free`, and raw pointers are generally not acceptable. + - cf. [C++ Core Guidelines, C.149: Use unique_ptr or shared_ptr to avoid forgetting to delete objects created using new](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c149-use-unique_ptr-or-shared_ptr-to-avoid-forgetting-to-delete-objects-created-using-new), [C++ Core Guidelines, R.10: Avoid malloc() and free()](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#r10-avoid-malloc-and-free) - When using `mutex`, use `std::lock_guard` instead of `std::lock` and `std::unlock`. + - cf. [C++ Core Guidelines, RAII](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rr-raii) - Use assert to validate that function arguments and return values meet specific conditions or expectations. - Be cautious when using `throw`. C++ exceptions should only be used when it's acceptable for the process to terminate if uncaught. This applies to scenarios where Autoware has no other choice but to shut down. + - cf. [Google C++ Style Guide, Exceptions](https://google.github.io/styleguide/cppguide.html#Exceptions) ## ROS 2 Style Guide @@ -48,6 +54,7 @@ As a basic rule, follow [the ROS 2 developer guide](https://docs.ros.org/en/humb - With `RCLCPP`, logs are automatically recorded to `/rosout`. These logs can be saved to a rosbag, which can then be replayed to review the log data. - Follow [the directory structure guideline](./ros-nodes/directory-structure.md). - If `RCLCPP_INFO` generates a large amount of logs, use `RCLCPP_INFO_THROTTLE`. + - cf. [Coding guidelines, ros-nodes, Use throttled logging when the log is unnecessarily shown repeatedly](./ros-nodes/console-logging.md#use-throttled-logging-when-the-log-is-unnecessarily-shown-repeatedly-required-non-automated) - Avoid setting default values in `declare_parameter`. Instead, specify them in a config file or launch file. ## Autoware Style Guide @@ -55,6 +62,8 @@ As a basic rule, follow [the ROS 2 developer guide](https://docs.ros.org/en/humb For Autoware-specific styles, refer to the following: - Use the `autoware_` prefix for package names. + - cf. [Prefix packages with autoware\_](https://github.com/orgs/autowarefoundation/discussions/4097) - Add implementations within the `autoware` namespace. + - cf. [Prefix packages with autoware\_, Option 3:](https://github.com/orgs/autowarefoundation/discussions/4097#discussioncomment-8384169) - In `CMakeLists.txt`, use `autoware_package()`. - cf. [autoware_cmake README](https://github.com/autowarefoundation/autoware_cmake/tree/main/autoware_cmake) From 00e9b4e71bdb50fb0faf6c30a00ebf078aafef1c Mon Sep 17 00:00:00 2001 From: Shintaro Tomie <58775300+Shin-kyoto@users.noreply.github.com> Date: Mon, 19 Aug 2024 22:17:16 +0900 Subject: [PATCH 05/14] docs(contributing): change the example variable's name to understand easily Co-authored-by: Yutaka Kondo --- docs/contributing/coding-guidelines/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing/coding-guidelines/index.md b/docs/contributing/coding-guidelines/index.md index 36c90b58471..cdc98dab8b8 100644 --- a/docs/contributing/coding-guidelines/index.md +++ b/docs/contributing/coding-guidelines/index.md @@ -29,7 +29,7 @@ As a basic rule, follow [the Google C++ Style Guide](https://google.github.io/st - Use fixed-width integer types like `uint32_t` instead of `int`. - cf. [How to C in 2016](https://matt.sh/howto-c) - Use `const` for variables whose values will not change after initialization, and for functions or arguments that guarantee they will not modify the object's state. This improves code safety and clarity by preventing unintended side effects. -- When not using an index, prefer `for (const auto & e : v)`. +- When not using an index, prefer `for (const auto & element : values)`. - cf. [C++ Core Guidelines, ES.71: Prefer a range-for-statement to a for-statement when there is a choice](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es71-prefer-a-range-for-statement-to-a-for-statement-when-there-is-a-choice) - For index access, prefer `.at(idx)` over `[idx]` whenever possible for safety. - cf. [C++ Core Guidelines, Avoid bounds errors](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#slcon3-avoid-bounds-errors) From 7943df0f7f13f4f3e717665d8efb97651a27dae1 Mon Sep 17 00:00:00 2001 From: Shintaro Tomie <58775300+Shin-kyoto@users.noreply.github.com> Date: Mon, 19 Aug 2024 22:20:28 +0900 Subject: [PATCH 06/14] docs(contributing): make the explanation about RCPCPP_* macro appropriate Co-authored-by: Yutaka Kondo --- docs/contributing/coding-guidelines/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing/coding-guidelines/index.md b/docs/contributing/coding-guidelines/index.md index cdc98dab8b8..6836bb40a6a 100644 --- a/docs/contributing/coding-guidelines/index.md +++ b/docs/contributing/coding-guidelines/index.md @@ -47,7 +47,7 @@ As a basic rule, follow [the Google C++ Style Guide](https://google.github.io/st As a basic rule, follow [the ROS 2 developer guide](https://docs.ros.org/en/humble/The-ROS2-Project/Contributing/Developer-Guide.html) and [the ROS nodes guidelines](./ros-nodes/class-design.md). The following are commonly pointed out areas that require attention: -- Use `RCLCPP` instead of `printf` or `std::cout`. +- Use `RCLCPP_XXX` (eg. `RCLCPP_INFO`) macros instead of `printf` or `std::cout` for logging. - Reasons include the following: - It allows for consistent log level management. For instance, with `RCLCPP`, you can simply set `--log_level` to adjust the log level uniformly across the application. - You can standardize the format using `RCUTILS_CONSOLE_OUTPUT_FORMAT`. From bfb5e20ba0124501634c00d15611ccf5a45db399 Mon Sep 17 00:00:00 2001 From: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> Date: Mon, 19 Aug 2024 22:24:06 +0900 Subject: [PATCH 07/14] docs(contributing): make the explanation for RCLCPP_* appropriate Signed-off-by: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> --- docs/contributing/coding-guidelines/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing/coding-guidelines/index.md b/docs/contributing/coding-guidelines/index.md index 6836bb40a6a..175ffcdc43a 100644 --- a/docs/contributing/coding-guidelines/index.md +++ b/docs/contributing/coding-guidelines/index.md @@ -47,7 +47,7 @@ As a basic rule, follow [the Google C++ Style Guide](https://google.github.io/st As a basic rule, follow [the ROS 2 developer guide](https://docs.ros.org/en/humble/The-ROS2-Project/Contributing/Developer-Guide.html) and [the ROS nodes guidelines](./ros-nodes/class-design.md). The following are commonly pointed out areas that require attention: -- Use `RCLCPP_XXX` (eg. `RCLCPP_INFO`) macros instead of `printf` or `std::cout` for logging. +- Use `RCLCPP_*` (e.g. `RCLCPP_INFO`) macros instead of `printf` or `std::cout` for logging. - Reasons include the following: - It allows for consistent log level management. For instance, with `RCLCPP`, you can simply set `--log_level` to adjust the log level uniformly across the application. - You can standardize the format using `RCUTILS_CONSOLE_OUTPUT_FORMAT`. From 2538ade66d1fb694f0133f73511954bcfb38dfcf Mon Sep 17 00:00:00 2001 From: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> Date: Mon, 19 Aug 2024 22:43:27 +0900 Subject: [PATCH 08/14] docs(contributing): add the guideline for directory structure of the header files to be exported Signed-off-by: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> --- docs/contributing/coding-guidelines/index.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/contributing/coding-guidelines/index.md b/docs/contributing/coding-guidelines/index.md index 175ffcdc43a..91356bc84ee 100644 --- a/docs/contributing/coding-guidelines/index.md +++ b/docs/contributing/coding-guidelines/index.md @@ -65,5 +65,7 @@ For Autoware-specific styles, refer to the following: - cf. [Prefix packages with autoware\_](https://github.com/orgs/autowarefoundation/discussions/4097) - Add implementations within the `autoware` namespace. - cf. [Prefix packages with autoware\_, Option 3:](https://github.com/orgs/autowarefoundation/discussions/4097#discussioncomment-8384169) +- The header files to be exported must be placed in the `PACKAGE_NAME/include/autoware/` directory. + - cf. [Directory structure guideline, Exporting headers](./ros-nodes/directory-structure.md#exporting-headers) - In `CMakeLists.txt`, use `autoware_package()`. - cf. [autoware_cmake README](https://github.com/autowarefoundation/autoware_cmake/tree/main/autoware_cmake) From 9f515d4bf06ac259130ca1246701aa0963d405d0 Mon Sep 17 00:00:00 2001 From: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> Date: Mon, 19 Aug 2024 22:57:56 +0900 Subject: [PATCH 09/14] docs(contributing): make the explanation about RCPCPP_* macro appropriate Signed-off-by: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> --- docs/contributing/coding-guidelines/index.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/contributing/coding-guidelines/index.md b/docs/contributing/coding-guidelines/index.md index 91356bc84ee..67164cb7f7d 100644 --- a/docs/contributing/coding-guidelines/index.md +++ b/docs/contributing/coding-guidelines/index.md @@ -49,9 +49,9 @@ As a basic rule, follow [the ROS 2 developer guide](https://docs.ros.org/en/humb - Use `RCLCPP_*` (e.g. `RCLCPP_INFO`) macros instead of `printf` or `std::cout` for logging. - Reasons include the following: - - It allows for consistent log level management. For instance, with `RCLCPP`, you can simply set `--log_level` to adjust the log level uniformly across the application. + - It allows for consistent log level management. For instance, with `RCLCPP_*` macros, you can simply set `--log_level` to adjust the log level uniformly across the application. - You can standardize the format using `RCUTILS_CONSOLE_OUTPUT_FORMAT`. - - With `RCLCPP`, logs are automatically recorded to `/rosout`. These logs can be saved to a rosbag, which can then be replayed to review the log data. + - With `RCLCPP_*` macros, logs are automatically recorded to `/rosout`. These logs can be saved to a rosbag, which can then be replayed to review the log data. - Follow [the directory structure guideline](./ros-nodes/directory-structure.md). - If `RCLCPP_INFO` generates a large amount of logs, use `RCLCPP_INFO_THROTTLE`. - cf. [Coding guidelines, ros-nodes, Use throttled logging when the log is unnecessarily shown repeatedly](./ros-nodes/console-logging.md#use-throttled-logging-when-the-log-is-unnecessarily-shown-repeatedly-required-non-automated) From 7f812d7b39fa2a8a4b395d5d6d20046ce67f2da8 Mon Sep 17 00:00:00 2001 From: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> Date: Tue, 20 Aug 2024 11:31:49 +0900 Subject: [PATCH 10/14] docs(contributing): use the appropriate format Signed-off-by: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> --- docs/contributing/coding-guidelines/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing/coding-guidelines/index.md b/docs/contributing/coding-guidelines/index.md index 67164cb7f7d..12f3662a5b4 100644 --- a/docs/contributing/coding-guidelines/index.md +++ b/docs/contributing/coding-guidelines/index.md @@ -40,7 +40,7 @@ As a basic rule, follow [the Google C++ Style Guide](https://google.github.io/st - When using `mutex`, use `std::lock_guard` instead of `std::lock` and `std::unlock`. - cf. [C++ Core Guidelines, RAII](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rr-raii) - Use assert to validate that function arguments and return values meet specific conditions or expectations. -- Be cautious when using `throw`. C++ exceptions should only be used when it's acceptable for the process to terminate if uncaught. This applies to scenarios where Autoware has no other choice but to shut down. +- Be cautious when using `throw`. C++ exceptions should only be used when it is acceptable for the process to terminate if uncaught. This applies to scenarios where Autoware has no other choice but to shut down. - cf. [Google C++ Style Guide, Exceptions](https://google.github.io/styleguide/cppguide.html#Exceptions) ## ROS 2 Style Guide From 0d2d9c5fa6784d29b45e153ab1b0b17c930293a9 Mon Sep 17 00:00:00 2001 From: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> Date: Tue, 20 Aug 2024 11:47:11 +0900 Subject: [PATCH 11/14] docs(contributing): add Autoware C++ coding guideline as a reference Signed-off-by: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> --- docs/contributing/coding-guidelines/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing/coding-guidelines/index.md b/docs/contributing/coding-guidelines/index.md index 12f3662a5b4..34ae6d94427 100644 --- a/docs/contributing/coding-guidelines/index.md +++ b/docs/contributing/coding-guidelines/index.md @@ -24,7 +24,7 @@ Also, keep in mind the following concepts. ## C++ Style Guide -As a basic rule, follow [the Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html) and [the C++ Core Guidelines](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md). The following are common areas requiring special attention: +As a basic rule, follow [the Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html), [the C++ Core Guidelines](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md) and [Autoware C++ Coding Guidelines](./languages/cpp.md). The following are common areas requiring special attention: - Use fixed-width integer types like `uint32_t` instead of `int`. - cf. [How to C in 2016](https://matt.sh/howto-c) From f3863e76f9d1bce9e95591c5113e1d6477ec2277 Mon Sep 17 00:00:00 2001 From: Shintaro Tomie <58775300+Shin-kyoto@users.noreply.github.com> Date: Tue, 20 Aug 2024 17:18:29 +0900 Subject: [PATCH 12/14] docs(contributing): use a specific numbers or reference for better understanding Co-authored-by: Takamasa Horibe --- docs/contributing/coding-guidelines/index.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/contributing/coding-guidelines/index.md b/docs/contributing/coding-guidelines/index.md index 34ae6d94427..bbcc66da122 100644 --- a/docs/contributing/coding-guidelines/index.md +++ b/docs/contributing/coding-guidelines/index.md @@ -15,9 +15,9 @@ Also, keep in mind the following concepts. - Keep things consistent. - Automate where possible, using simple checks for formatting, syntax, etc. - Write comments and documentation in English. -- Functions that are too complex (low cohesion) should be appropriately split into smaller functions. +- Functions that are too complex (low cohesion) should be appropriately split into smaller functions (e.g. less than 40 lines in one function is recommended in the [google style guideline](https://google.github.io/styleguide/cppguide.html#Write_Short_Functions)). - Try to minimize the use of member variables or global variables that have a large scope. -- Whenever possible, break large pull requests into smaller, manageable PRs. +- Whenever possible, break large pull requests into smaller, manageable PRs (less than 200 lines of change is recommended in some research e.g. [here](https://opensource.com/article/18/6/anatomy-perfect-pull-request)). - When it comes to code reviews, don't spend too much time on trivial disagreements. For details see: - - From 289937d37b92d3c87934738029adce2e975ee108 Mon Sep 17 00:00:00 2001 From: t4-adc Date: Fri, 23 Aug 2024 18:42:27 +0900 Subject: [PATCH 13/14] docs(contributing): split guidelines into separate files for each language and removed the content already covered in each language-specific guideline. Signed-off-by: t4-adc --- docs/contributing/coding-guidelines/index.md | 39 ++----------------- .../coding-guidelines/languages/cpp.md | 23 +++++++++++ 2 files changed, 27 insertions(+), 35 deletions(-) diff --git a/docs/contributing/coding-guidelines/index.md b/docs/contributing/coding-guidelines/index.md index bbcc66da122..62263ad4c1e 100644 --- a/docs/contributing/coding-guidelines/index.md +++ b/docs/contributing/coding-guidelines/index.md @@ -21,41 +21,10 @@ Also, keep in mind the following concepts. - When it comes to code reviews, don't spend too much time on trivial disagreements. For details see: - - - -## C++ Style Guide - -As a basic rule, follow [the Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html), [the C++ Core Guidelines](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md) and [Autoware C++ Coding Guidelines](./languages/cpp.md). The following are common areas requiring special attention: - -- Use fixed-width integer types like `uint32_t` instead of `int`. - - cf. [How to C in 2016](https://matt.sh/howto-c) -- Use `const` for variables whose values will not change after initialization, and for functions or arguments that guarantee they will not modify the object's state. This improves code safety and clarity by preventing unintended side effects. -- When not using an index, prefer `for (const auto & element : values)`. - - cf. [C++ Core Guidelines, ES.71: Prefer a range-for-statement to a for-statement when there is a choice](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es71-prefer-a-range-for-statement-to-a-for-statement-when-there-is-a-choice) -- For index access, prefer `.at(idx)` over `[idx]` whenever possible for safety. - - cf. [C++ Core Guidelines, Avoid bounds errors](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#slcon3-avoid-bounds-errors) -- Avoid using auto with `Eigen::Matrix` or `Eigen::Vector` variables, as it can lead to bugs. - - cf. [Eigen, C++11 and the auto keyword](https://eigen.tuxfamily.org/dox/TopicPitfalls.html) -- Safe memory management is crucial for accident-free autonomous vehicles, so `new`, `delete`, `malloc`, `free`, and raw pointers are generally not acceptable. - - cf. [C++ Core Guidelines, C.149: Use unique_ptr or shared_ptr to avoid forgetting to delete objects created using new](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c149-use-unique_ptr-or-shared_ptr-to-avoid-forgetting-to-delete-objects-created-using-new), [C++ Core Guidelines, R.10: Avoid malloc() and free()](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#r10-avoid-malloc-and-free) -- When using `mutex`, use `std::lock_guard` instead of `std::lock` and `std::unlock`. - - cf. [C++ Core Guidelines, RAII](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rr-raii) -- Use assert to validate that function arguments and return values meet specific conditions or expectations. -- Be cautious when using `throw`. C++ exceptions should only be used when it is acceptable for the process to terminate if uncaught. This applies to scenarios where Autoware has no other choice but to shut down. - - cf. [Google C++ Style Guide, Exceptions](https://google.github.io/styleguide/cppguide.html#Exceptions) - -## ROS 2 Style Guide - -As a basic rule, follow [the ROS 2 developer guide](https://docs.ros.org/en/humble/The-ROS2-Project/Contributing/Developer-Guide.html) and [the ROS nodes guidelines](./ros-nodes/class-design.md). The following are commonly pointed out areas that require attention: - -- Use `RCLCPP_*` (e.g. `RCLCPP_INFO`) macros instead of `printf` or `std::cout` for logging. - - Reasons include the following: - - It allows for consistent log level management. For instance, with `RCLCPP_*` macros, you can simply set `--log_level` to adjust the log level uniformly across the application. - - You can standardize the format using `RCUTILS_CONSOLE_OUTPUT_FORMAT`. - - With `RCLCPP_*` macros, logs are automatically recorded to `/rosout`. These logs can be saved to a rosbag, which can then be replayed to review the log data. -- Follow [the directory structure guideline](./ros-nodes/directory-structure.md). -- If `RCLCPP_INFO` generates a large amount of logs, use `RCLCPP_INFO_THROTTLE`. - - cf. [Coding guidelines, ros-nodes, Use throttled logging when the log is unnecessarily shown repeatedly](./ros-nodes/console-logging.md#use-throttled-logging-when-the-log-is-unnecessarily-shown-repeatedly-required-non-automated) -- Avoid setting default values in `declare_parameter`. Instead, specify them in a config file or launch file. +- Please follow the guidelines for each language. + - [C++](./languages/cpp.md) + - [Python](./languages/python.md) + - [Shell script](./languages/shell-scripts.md) ## Autoware Style Guide diff --git a/docs/contributing/coding-guidelines/languages/cpp.md b/docs/contributing/coding-guidelines/languages/cpp.md index ab4d39e4cbf..da9383a377e 100644 --- a/docs/contributing/coding-guidelines/languages/cpp.md +++ b/docs/contributing/coding-guidelines/languages/cpp.md @@ -202,3 +202,26 @@ constexpr double gravity = 9.80665; class RosApi; RosApi ros_api; ``` + +### Do not use the auto keywords with Eigen's expressions (required) + +#### Rationale + +- Avoid using auto with `Eigen::Matrix` or `Eigen::Vector` variables, as it can lead to bugs. + +#### Reference + +- [Eigen, C++11 and the auto keyword](https://eigen.tuxfamily.org/dox/TopicPitfalls.html) + +### Use RCLCPP\_\* (e.g. RCLCPP_INFO) macros instead of printf or std::cout for logging (required) + +#### Rationale + +- Reasons include the following: + - It allows for consistent log level management. For instance, with RCLCPP\_\* macros, you can simply set --log_level to adjust the log level uniformly across the application. + - You can standardize the format using RCUTILS_CONSOLE_OUTPUT_FORMAT. + - With RCLCPP\_\* macros, logs are automatically recorded to /rosout. These logs can be saved to a rosbag, which can then be replayed to review the log data. + +#### Reference + +- [Autoware Documentation for Console logging in ROS Node](../ros-nodes/console-logging.md) From eeee09d06bdd1ce88232bd47e8098b090bdc4bfc Mon Sep 17 00:00:00 2001 From: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> Date: Sun, 1 Sep 2024 01:49:22 +0900 Subject: [PATCH 14/14] docs(contributing): add reference to autoware style guideline Signed-off-by: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com> --- docs/contributing/coding-guidelines/index.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/contributing/coding-guidelines/index.md b/docs/contributing/coding-guidelines/index.md index 62263ad4c1e..0b12f7b93a5 100644 --- a/docs/contributing/coding-guidelines/index.md +++ b/docs/contributing/coding-guidelines/index.md @@ -31,8 +31,10 @@ Also, keep in mind the following concepts. For Autoware-specific styles, refer to the following: - Use the `autoware_` prefix for package names. + - cf. [Directory structure guideline, Package name](./ros-nodes/directory-structure.md#package-name) - cf. [Prefix packages with autoware\_](https://github.com/orgs/autowarefoundation/discussions/4097) - Add implementations within the `autoware` namespace. + - cf. [Class design guideline, Namespaces](./ros-nodes/class-design.md#namespaces) - cf. [Prefix packages with autoware\_, Option 3:](https://github.com/orgs/autowarefoundation/discussions/4097#discussioncomment-8384169) - The header files to be exported must be placed in the `PACKAGE_NAME/include/autoware/` directory. - cf. [Directory structure guideline, Exporting headers](./ros-nodes/directory-structure.md#exporting-headers)