Skip to content

Document memory_conf_t and memory_rule_t members#30

Merged
Acuadros95 merged 3 commits intomicro-ROS:galacticfrom
gavanderhoorn:patch-1
May 13, 2022
Merged

Document memory_conf_t and memory_rule_t members#30
Acuadros95 merged 3 commits intomicro-ROS:galacticfrom
gavanderhoorn:patch-1

Conversation

@gavanderhoorn
Copy link
Copy Markdown
Contributor

As per subject.

I've added some (very terse) documentation to the fields in micro_ros_utilities_memory_rule_t and micro_ros_utilities_memory_conf_t, as I kept having to go back to the Handling messages memory in micro-ROS page to see what the fields are for.

Note: I'm not sure I understood the purpose of each of the fields completely -- or documented them correctly. The purpose of this PR is as much a way for me to get some confirmation.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #30 (49da026) into galactic (327213a) will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           galactic      #30      +/-   ##
============================================
  Coverage     60.84%   60.84%              
============================================
  Files           620       10     -610     
  Lines         64604     1042   -63562     
  Branches      27776      448   -27328     
============================================
- Hits          39308      634   -38674     
+ Misses         7688      124    -7564     
+ Partials      17608      284   -17324     
Impacted Files Coverage Δ
...utilities/test/micro_ros_utilities/test_string.cpp
...ibc74rbsb/micro_ros_utilities/src/type_utilities.c
...utilities/test/micro_ros_utilities/test_memory.hpp
...utilities/test/micro_ros_utilities/test_memory.cpp
...utilities/test/micro_ros_utilities/test_memory.cpp
...utilities/test/micro_ros_utilities/test_string.cpp
...utilities/test/micro_ros_utilities/test_string.cpp
...utilities/test/micro_ros_utilities/test_memory.cpp
...1r9jr58rl/micro_ros_utilities/src/type_utilities.c
...utilities/test/micro_ros_utilities/test_string.cpp
... and 620 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 327213a...49da026. Read the comment docs.

Comment thread include/micro_ros_utilities/type_utilities.h Outdated
Comment thread include/micro_ros_utilities/type_utilities.h Outdated
Comment thread include/micro_ros_utilities/type_utilities.h
@Acuadros95 Acuadros95 requested a review from pablogs9 May 9, 2022 07:30
@Acuadros95
Copy link
Copy Markdown
Contributor

Just a few comments, also adding @pablogs9 as a reviewer.

Copy link
Copy Markdown
Member

@pablogs9 pablogs9 left a comment

Choose a reason for hiding this comment

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

I'm OK with this if @Acuadros95 comments are added. Ping @gavanderhoorn

@gavanderhoorn
Copy link
Copy Markdown
Contributor Author

Friendly ping?

@pablogs9
Copy link
Copy Markdown
Member

Please @gavanderhoorn could you add the requested changes?

@gavanderhoorn
Copy link
Copy Markdown
Contributor Author

If you could take a look at my responses to your comments :)

gavanderhoorn and others added 2 commits May 13, 2022 10:50
Co-authored-by: Antonio Cuadros <49162117+Acuadros95@users.noreply.github.com>
Co-authored-by: Antonio Cuadros <49162117+Acuadros95@users.noreply.github.com>
@gavanderhoorn
Copy link
Copy Markdown
Contributor Author

I've just accepted your proposal, as I couldn't come up with a satisfying alternative phrasing.

@Acuadros95 Acuadros95 merged commit 3df7cf5 into micro-ROS:galactic May 13, 2022
@Acuadros95
Copy link
Copy Markdown
Contributor

@mergify backport main

mergify Bot pushed a commit that referenced this pull request May 13, 2022
* Document memory_conf_t and memory_rule_t members

* Fix typo

Co-authored-by: Antonio Cuadros <49162117+Acuadros95@users.noreply.github.com>

* Update include/micro_ros_utilities/type_utilities.h

Co-authored-by: Antonio Cuadros <49162117+Acuadros95@users.noreply.github.com>

Co-authored-by: Antonio Cuadros <49162117+Acuadros95@users.noreply.github.com>
(cherry picked from commit 3df7cf5)
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 13, 2022

backport main

✅ Backports have been created

Details

Acuadros95 pushed a commit that referenced this pull request May 13, 2022
* Document memory_conf_t and memory_rule_t members

* Fix typo

Co-authored-by: Antonio Cuadros <49162117+Acuadros95@users.noreply.github.com>

* Update include/micro_ros_utilities/type_utilities.h

Co-authored-by: Antonio Cuadros <49162117+Acuadros95@users.noreply.github.com>

Co-authored-by: Antonio Cuadros <49162117+Acuadros95@users.noreply.github.com>
(cherry picked from commit 3df7cf5)

Co-authored-by: G.A. vd. Hoorn <g.a.vanderhoorn@tudelft.nl>
@gavanderhoorn gavanderhoorn deleted the patch-1 branch May 13, 2022 09:31
@gavanderhoorn
Copy link
Copy Markdown
Contributor Author

thanks

@pablogs9
Copy link
Copy Markdown
Member

pablogs9 commented May 17, 2022

This seems to have break the CI in the API doc generator: https://github.com/micro-ROS/micro-ROS.github.io/runs/6465261539?check_suite_focus=true

Can you take a look @Acuadros95 ?

@Acuadros95
Copy link
Copy Markdown
Contributor

@pablogs9 Fixed on #34

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.

4 participants