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

Documentation linter #295

Open
ahcorde opened this issue Jul 24, 2020 · 5 comments
Open

Documentation linter #295

ahcorde opened this issue Jul 24, 2020 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@ahcorde
Copy link

ahcorde commented Jul 24, 2020

This issue is to define how we want to check that all API is documented. In particular to check if there are undocumented functions, parameters or attributes in the code.

I think that make sense to include another linter inside the ament_lint package.

  • ament_doxygen: It will find the Doxygen executable in your system, it will generate the documentation and expose the potencial warnings.
  • ament_cmake_doxygen: It allows to use ament_doxygen package inside CMake and it will call the linter during the test step.

Right now, all these packages contains a Doxyfile in the source repositories : action_msgs, ament_index_cpp, builtin_interfaces, class_loader, composition_interfaces, console_bridge_vendor, diagnostic_msgs, foonathan_memory_vendor, geometry_msgs, libconsole-bridge-dev, libstatistics_collector, libyaml_vendor, lifecycle_msgs, nav_msgs, rcl, rcl_action, rcl_interfaces, rcl_lifecycle, rcl_logging_spdlog, rcl_yaml_param_parser, rclcpp, rclcpp_action, rclcpp_components, rclcpp_lifecycle, rcpputils, rcutils, rmw, rmw_dds_common, rmw_fastrtps_cpp, rmw_fastrtps_shared_cpp, rmw_implementation, rosgraph_msgs, rosidl_default_runtime, rosidl_runtime_c, rosidl_runtime_cpp, rosidl_typesupport_c, rosidl_typesupport_cpp, rosidl_typesupport_fastrtps_c, rosidl_typesupport_fastrtps_cpp, rosidl_typesupport_interface, sensor_msgs, shape_msgs, spdlog_vendor, statistics_msgs, std_msgs, std_srvs, stereo_msgs, trajectory_msgs, unique_identifier_msgs and visualization_msg.

I was reviewing the current implementation in ROS. It uses rosdoc_lite which is a tool that uses a file called rosdoc.yaml to generate the documentation. If this file contains an entry for Doxygen (builder: doxygen) them it will create a Doxyfile based on a template.

Some of the ROS 2 packages generate some code that it's installed in the install folder. Right now, these generated files are not included in the Doxyfile, the Doxyfile it's desing to execute it inside the package's source directory.

Explained the context my suggestion are:

  • Remove the Doxyfile from the source repositories and this rosdoc.yaml file.
  • Create a tool for ROS 2 that allows to generate the Doxyfile based on the rosdoc.yaml file (to point the installed folder which and the end it's the available public API). This tool can be part of the ament_doxygen package.

This setup I think it's more scalable if decide to change the Doxygen template then we don't need to commit in every repository and it will allow to point the installed folder.

@ahcorde ahcorde added the enhancement New feature or request label Jul 24, 2020
@ahcorde ahcorde self-assigned this Jul 24, 2020
@gavanderhoorn
Copy link
Contributor

This issue is to define how we want to check that all API is documented. In particular to check if there are undocumented functions, parameters or attributes in the code.

Are you planning to use some tooling for this?

I've used this (with varying success) in the past: psycofdj/coverxygen.

@jacobperron
Copy link
Member

If the proposed ament_doxygen tool is going to do more than documentation related linter checks (e.g. actually generate HTML documentation), then it can probably live in it's own repository instead of ament_lint.

I wonder if we really need a rosdoc.yaml file. Couldn't the same information be given to the proposed ament_doxygen command? I'm not sure what this looks like in the context of a pure Python package (or if pure Python packages are in scope).

This setup I think it's more scalable if decide to change the Doxygen template then we don't need to commit in every repository and it will allow to point the installed folder.

It might also be valuable to let packages supply their own Doxygen files (e.g. override the default provided by ament_doxygen).

@ahcorde
Copy link
Author

ahcorde commented Jul 29, 2020

I have been reading the article "ROS 2 Documentation System" which is not public yet, bit it's available in the ros2/desing repository. This article describes the proposed system for doing documentation for ROS 2. It has some TODOs and it was original wrote by @wjwwood , I don't know what is the status/relevance of this article because last commit was done 3 Feb 2016. Can you provide a quick update about this article? or links to futher discussions?

Another maybe related topic is in the REP 149 (Package Manifest Format Three Specification) exists a field called <doc_depend> but the REP indicates that this is unused right now:

The current version of the buildsystem does not provide any documentation specific functionality

Is there any ideas about how this field should be used in the future? Because this field may help with the tags in Doxygen.

@dirk-thomas
Copy link
Member

The current version of the buildsystem does not provide any documentation specific functionality

While the build system doesn't utilize doc dependencies atm the buildfarm does within the ROS 1 doc jobs.

@ahcorde
Copy link
Author

ahcorde commented Jul 29, 2020

I'm not sure what this looks like in the context of a pure Python package (or if pure Python packages are in scope).

I started doing this for the Quality Level 1 packages which are mainly C++, I think that Python uses Sphinx to generate the documentation. Maybe I can generalize this new linter and call it ament_lint_doc to include both engines.

It might also be valuable to let packages supply their own Doxygen files (e.g. override the default provided by ament_doxygen).

For packages such as rclcpp that generates some files, we should override or at least generate a new Doxyfile on in the build directory with the right path to the installed include folder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants