Skip to content

Apply feedback from Javi, Mich and Gera#31

Merged
JesusSilvaUtrera merged 8 commits intomainfrom
feature/apply-feedback
Oct 28, 2025
Merged

Apply feedback from Javi, Mich and Gera#31
JesusSilvaUtrera merged 8 commits intomainfrom
feature/apply-feedback

Conversation

@JesusSilvaUtrera
Copy link
Copy Markdown
Collaborator

@JesusSilvaUtrera JesusSilvaUtrera commented Oct 27, 2025

What this PR does

Apply the feedback received from Javi and Mich.

Points that weren't tackled:

  • Adding a video and/or rosbag in Module 5
  • Adding RTest example (doesn't seem so relevant to me right now)
  • Modules 5 and 6 are lacking code
  • Provide an example that illustrates usage of EXPECT vs ASSERT
  • Vomment on the difference between using pre-commit hooks vs colcon triggered linting

Type

  • Bugfix
  • Feature
  • Documentation

How to test

Steps to reproduce / test the changes:

  1. ...
  2. ...
  3. ...

Checklist

  • I have signed my commits (git commit -s) or added Signed-off-by to existing commits.
  • I added/updated tests (if applicable)
  • I updated documentation (if applicable)

@JesusSilvaUtrera
Copy link
Copy Markdown
Collaborator Author

I still need to refine the solution for Exercise 4, I changed a bit the exercise using Mich's feedback

@JesusSilvaUtrera
Copy link
Copy Markdown
Collaborator Author

specific-ci passed when it shouldn't, but tests are failing inside if you take a look to the logs

@JesusSilvaUtrera
Copy link
Copy Markdown
Collaborator Author

I still need to refine the solution for Exercise 4, I changed a bit the exercise using Mich's feedback

Tested, new solution is working

Copy link
Copy Markdown
Collaborator

@xaru8145 xaru8145 left a comment

Choose a reason for hiding this comment

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

Minor things. PTAL

Comment thread modules/module_4/README.md Outdated
Comment thread modules/module_4/README.md Outdated
Comment thread modules/module_2/README.md Outdated
Comment thread modules/module_1/README.md Outdated
@xaru8145 xaru8145 changed the title Apply feedback from Javi and Mich Apply feedback from Javi, Mich and Gera Oct 27, 2025
Comment thread tools/run_ci_build.sh
@xaru8145 xaru8145 force-pushed the feature/apply-feedback branch from 394fc0b to 22e6643 Compare October 27, 2025 22:27
Comment thread modules/module_1/README.md Outdated
Comment thread modules/module_1/README.md Outdated
Comment thread README.md Outdated
Copy link
Copy Markdown
Collaborator

@xaru8145 xaru8145 left a comment

Choose a reason for hiding this comment

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

LGTM

@JesusSilvaUtrera JesusSilvaUtrera merged commit 0da3412 into main Oct 28, 2025
0 of 2 checks passed
@JesusSilvaUtrera JesusSilvaUtrera deleted the feature/apply-feedback branch October 28, 2025 11:06
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