Skip to content

Conversation

@mxgrey
Copy link

@mxgrey mxgrey commented Nov 19, 2024

This PR addresses some of the feedback on the upstream logging PR.

This can be safely squash-merged after #12 does a commit-merge, or you can commit-merge this PR and close #12

mxgrey and others added 8 commits October 30, 2024 12:59
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <greyxmike@gmail.com>
* Use latest stable Rust CI

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>

* Fix quotation in doc

Signed-off-by: Michael X. Grey <grey@openrobotics.org>

* Fix serde feature for vendored messages

Signed-off-by: Michael X. Grey <grey@openrobotics.org>

* Fix formatting of lists in docs

Signed-off-by: Michael X. Grey <grey@openrobotics.org>

* Update minimum supported Rust version based on the currently used language features

Signed-off-by: Michael X. Grey <grey@openrobotics.org>

* Conform to clippy style guide

Signed-off-by: Michael X. Grey <grey@openrobotics.org>

* Add rust toolchain as a matrix dimension

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>

* Bump declaration of minimum supported rust version

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>

* Limit the scheduled runs to once a week

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>

* Define separate stable and minimal workflows because matrix does not work with reusable actions

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>

* Reduce minimum version to 1.75 to target Noble

Signed-off-by: Michael X. Grey <grey@openrobotics.org>

---------

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
* Update rclrs vendor_interfaces.py script

This updates the vendor_interfaces.py script to also vendor in the
action_msgs and unique_identifier_msgs packages. The script is modified
to always use a list of package names rather than hard-coding the
package names everywhere.

* Silence certain clippy lints on generated code

In case a user enforces clippy linting on these generated packages,
silence expected warnings. This is already the case in rclrs, but should
be applied directly to the generated packages for the sake of downstream
users.

The `clippy::derive_partial_eq_without_eq` lint was already being
disabled for the packages vendored by rclrs, but is now moved to the
rosidl_generator_rs template instead. This is necessary since we always
derive the PartialEq trait, but can't necessary derive Eq, and so don't.

The `clippy::upper_case_acronyms` is new and was added to account for
message type names being upper-case acrynyms, like
unique_identifier_msgs::msg::UUID.

* Update vendored message packages

This updates the message packages vendored under the rclrs `vendor`
module. The vendor_interfaces.py script was used for reproducibility.

The existing packages – rcl_interfaces, rosgraph_msgs, and
builtin_interfaces – are only modified in that they now use the
std::ffi::c_void naming for c_void instead of the std::os::raw::c_void
alias and disable certain clippy lints.

The action_msgs and unique_identifier_msgs packages are newly added to
enable adding action support to rclrs. action_msgs is needed for
interaction with action goal info and statuses, and has a dependency on
unique_identifier_msgs.
Signed-off-by: Michael X. Grey <greyxmike@gmail.com>
Signed-off-by: Michael X. Grey <greyxmike@gmail.com>
Signed-off-by: Michael X. Grey <greyxmike@gmail.com>
Signed-off-by: Michael X. Grey <greyxmike@gmail.com>
Signed-off-by: Michael X. Grey <greyxmike@gmail.com>
Signed-off-by: Michael X. Grey <greyxmike@gmail.com>
@geoff-imdex geoff-imdex merged commit 622e887 into geoff-imdex:main Nov 20, 2024
6 checks passed
@mxgrey mxgrey mentioned this pull request Nov 20, 2024
@geoff-imdex
Copy link
Owner

@mxgrey - that should be a lot nicer for everybody to work with; thank you

@mxgrey mxgrey deleted the pr_feedback branch November 20, 2024 11:24
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.

3 participants