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

Standardize Doxygen parameter formatting for systems A-N #2183

Merged
merged 4 commits into from
Oct 28, 2023

Conversation

mabelzhang
Copy link
Contributor

🦟 Bug fix

We were looking to improve the Doxygen pages for system plugins and noticed some huge paragraphs that meant to be bullet lists, and inconsistencies in headings.

This PR standardizes the formatting for SDF parameters for systems starting with letter A to N (which is not to say I will do the same for O to Z, as this isn't really the task I'm tackling... just fixing on the way as I familiarize myself with how systems are documented).
A-N is 46 systems. There are only 24 remaining that others can fight over.

Summary

This is the standard being adopted (comes from AckermannSteering, not because it's the first in the alphabetical list, but because its rendered page looks the best):

  • Heading size should be ## System Parameters, to appear the same size as the preset "Detailed Description" heading
  • Parameters should be in bullets, otherwise there's a variety of rendered results, especially when you have sub-bullets too, or if you do a \sa, or they are all in one huge paragraph.
  • Parameter font should be in code face, ideally surrounded by angle brackets, e.g. - <param>
  • Most parameters are followed by a colon, as in - <param>:, but I let some pass with hyphen, because it wasn't worth the effort.
  • I wasn't too picky about indentation after the first line, because it wasn't worth the effort

To test

colcon build --packages-select gz-sim8

In a browser, navigate to build/gz-sim8/doxygen/html/index.html
Click on the “gz::sim::systems” link.
Look at the corresponding system pages.

Compare to the live render here before the fix: https://gazebosim.org/api/sim/8/

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

…his is for systems beginning with A-N.

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
@mabelzhang mabelzhang added the documentation Improvements or additions to documentation label Sep 30, 2023
@mabelzhang mabelzhang marked this pull request as ready for review September 30, 2023 08:00
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Sep 30, 2023
@codecov
Copy link

codecov bot commented Sep 30, 2023

Codecov Report

Merging #2183 (414c063) into gz-sim8 (8c2fb9b) will increase coverage by 0.11%.
Report is 1 commits behind head on gz-sim8.
The diff coverage is n/a.

❗ Current head 414c063 differs from pull request most recent head 5134d21. Consider uploading reports for the commit 5134d21 to get more accurate results

@@             Coverage Diff             @@
##           gz-sim8    #2183      +/-   ##
===========================================
+ Coverage    65.92%   66.04%   +0.11%     
===========================================
  Files          323      323              
  Lines        30719    30718       -1     
===========================================
+ Hits         20251    20287      +36     
+ Misses       10468    10431      -37     
Files Coverage Δ
...rc/systems/ackermann_steering/AckermannSteering.hh 100.00% <ø> (ø)
src/systems/breadcrumbs/Breadcrumbs.hh 100.00% <ø> (ø)
src/systems/buoyancy/Buoyancy.hh 100.00% <ø> (ø)
src/systems/detachable_joint/DetachableJoint.hh 100.00% <ø> (ø)
src/systems/diff_drive/DiffDrive.hh 100.00% <ø> (ø)
src/systems/elevator/Elevator.hh 100.00% <ø> (ø)
src/systems/joint_controller/JointController.hh 100.00% <ø> (ø)
...int_position_controller/JointPositionController.hh 100.00% <ø> (ø)
...stems/joint_state_publisher/JointStatePublisher.hh 100.00% <ø> (ø)
...ms/joint_traj_control/JointTrajectoryController.hh 100.00% <ø> (ø)
... and 3 more

... and 2 files with indirect coverage changes

ahcorde
ahcorde previously requested changes Oct 2, 2023
src/systems/acoustic_comms/AcousticComms.hh Show resolved Hide resolved
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

Other than the @ahcorde's unresolved issue, LGTM!

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
@mabelzhang
Copy link
Contributor Author

Addressed

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
@mabelzhang
Copy link
Contributor Author

mabelzhang commented Oct 27, 2023

@ahcorde Merging is blocked until change requested changes to green check :)

@mjcarroll mjcarroll merged commit d693df6 into gz-sim8 Oct 28, 2023
5 of 8 checks passed
@mjcarroll mjcarroll deleted the mabelzhang/system_docs_format branch October 28, 2023 03:36
mabelzhang added a commit that referenced this pull request Nov 1, 2023
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation 🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants