Skip to content

Fix pluralization in ros2 trace output - #169

Merged
christophebedard merged 2 commits into
ros2:rollingfrom
devashravan:rolling
Apr 26, 2025
Merged

Fix pluralization in ros2 trace output#169
christophebedard merged 2 commits into
ros2:rollingfrom
devashravan:rolling

Conversation

@devashravan

Copy link
Copy Markdown
Contributor

This PR fixes a small issue in the output of the ros2 trace command. Earlier, even when only one event was enabled, the terminal would display:

userspace tracing enabled (1 events)

This has now been updated to correctly handle singular and plural forms:

userspace tracing enabled (1 event)

I’ve tested this locally using a source build of ROS 2 Rolling. Attached below is the terminal output for reference.

Let me know if any changes are needed!

Screenshot from 2025-04-25 02-45-11

Signed-off-by: Shravan Deva <devashravan7@gmail.com>

@christophebedard christophebedard left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR! I just have one minor comment.

Comment thread tracetools_trace/tracetools_trace/trace.py Outdated
@christophebedard

Copy link
Copy Markdown
Member

Also, the GitHub CI jobs are failing, but that's unrelated to your changes (see #160 and #170). What really matters is the CI jobs that I will manually trigger once the PR is ready.

Signed-off-by: Shravan Deva <devashravan7@gmail.com>
@devashravan

Copy link
Copy Markdown
Contributor Author

Hi @christophebedard , I've corrected the mistake you pointed out. Can you please review these changes?

@christophebedard christophebedard left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, thanks for the PR and for making the change!

@christophebedard

christophebedard commented Apr 25, 2025

Copy link
Copy Markdown
Member

Pulls: #169
Gist: https://gist.githubusercontent.com/christophebedard/9c220883dc2c8ad873bb99ecc9dbd7eb/raw/7e7e2fd0d824e737adaa2b3e94d5aee98eb86ca7/ros2.repos
BUILD args: --packages-above-and-dependencies tracetools_trace
TEST args: --packages-above tracetools_trace
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15794

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@devashravan

Copy link
Copy Markdown
Contributor Author

Hi @christophebedard , it looks like the Windows build is failing, but I don’t think it’s related to the changes I made in this PR. Could you please confirm when you get a chance?

@christophebedard

Copy link
Copy Markdown
Member

Yeah, that's related to https://github.com/ros2-rust/rosidl_rust. I retriggered the Windows job. Let's see if their new fix works (ros2-rust/rosidl_rust#3), otherwise I'll have to trigger a fresh set of CI jobs so that this repo is not included (ros2/ros2#1677).

@devashravan

Copy link
Copy Markdown
Contributor Author

Got it! I'll use this opportunity to learn more about GitHub CI.

@christophebedard

Copy link
Copy Markdown
Member

Alright, ROS 2 CI passed, so this is ready! Thanks again for the PR.

@christophebedard
christophebedard merged commit 01d969a into ros2:rolling Apr 26, 2025
RaphvK pushed a commit to RaphvK/ros2_tracing that referenced this pull request Mar 2, 2026
Signed-off-by: Shravan Deva <devashravan7@gmail.com>
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.

2 participants