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.

Sign up for GitHub

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

Jump to bottom

[21186] Examples refactor: Topic Instances #5033

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

[21186] Examples refactor: Topic Instances #5033

wants to merge 25 commits into from

Conversation

JesusPoderoso
Copy link
Contributor

@JesusPoderoso JesusPoderoso commented Jul 4, 2024 •
edited by elianalf
Loading

Description

This PR is part of a suite of PR which would make a refactor in the repository examples.
It is intended to apply to most of the examples, by making them homogeneous, more understandable, and more specific to the case they were meant to be.

In this topic instances example, the key changes are:

  • shape type compatible with shapesdemo
  • use of keyed topics and instances
  • configuration of shape parameters

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
  • Any new/modified methods have been properly documented using Doxygen.
  • N/A Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
  • ❌ Changes are backport compatible: they do NOT break ABI nor change library core behavior.
  • ❌ Changes are API compatible.
  • New feature has been added to the versions.md file (if applicable).
  • N/A New feature has been documented/Current behavior is correctly described in the documentation.
  • N/A Applicable backports have been included in the description.

Reviewer Checklist

  • The PR has a milestone assigned.
  • The title and description correctly express the PR's purpose.
  • Check contributor checklist is correct.
  • N/A If this is a critical bug fix, backports to the critical-only supported branches have been requested.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@JesusPoderoso JesusPoderoso added this to the v3.0.0 milestone Jul 4, 2024
@JesusPoderoso JesusPoderoso self-assigned this Jul 4, 2024
@github-actions github-actions bot added the ci-pending PR which CI is running label Jul 4, 2024
@JesusPoderoso JesusPoderoso requested review from elianalf and removed request for elianalf July 4, 2024 15:22
@JesusPoderoso JesusPoderoso force-pushed the feature/example_refactor/topic_instances branch from 142b0db to f7a4cd6 Compare July 4, 2024 15:26
@JesusPoderoso JesusPoderoso requested review from elianalf and removed request for elianalf July 4, 2024 15:26
@JesusPoderoso JesusPoderoso force-pushed the feature/example_refactor/topic_instances branch from 5293219 to 74fe642 Compare July 9, 2024 15:24
@JesusPoderoso JesusPoderoso requested review from elianalf and removed request for elianalf July 9, 2024 15:24
@JesusPoderoso JesusPoderoso force-pushed the feature/example_refactor/topic_instances branch from 74fe642 to b82d719 Compare July 10, 2024 10:21
@JesusPoderoso JesusPoderoso requested review from elianalf and removed request for elianalf July 10, 2024 10:21
@JesusPoderoso JesusPoderoso force-pushed the feature/example_refactor/topic_instances branch from b82d719 to 74e200c Compare July 10, 2024 11:42
@JesusPoderoso JesusPoderoso requested review from elianalf and removed request for elianalf July 10, 2024 11:42
elianalf
elianalf approved these changes Jul 11, 2024
View reviewed changes
Copy link
Contributor

@elianalf elianalf left a comment

Choose a reason for hiding this comment

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

Partial revision

examples/cpp/topic_instances/README.md Outdated Show resolved Hide resolved
examples/cpp/topic_instances/README.md Outdated Show resolved Hide resolved
examples/cpp/topic_instances/README.md Outdated Show resolved Hide resolved
examples/cpp/topic_instances/README.md Outdated Show resolved Hide resolved
examples/cpp/topic_instances/README.md Outdated Show resolved Hide resolved
examples/cpp/topic_instances/CLIParser.hpp Show resolved Hide resolved
examples/cpp/topic_instances/CLIParser.hpp Show resolved Hide resolved
examples/cpp/topic_instances/PublisherApp.hpp Show resolved Hide resolved
examples/cpp/topic_instances/CLIParser.hpp Outdated Show resolved Hide resolved
examples/cpp/topic_instances/PublisherApp.hpp Outdated Show resolved Hide resolved
elianalf
elianalf requested changes Jul 12, 2024
View reviewed changes
examples/cpp/topic_instances/PublisherApp.cpp Outdated Show resolved Hide resolved
examples/cpp/topic_instances/PublisherApp.cpp Show resolved Hide resolved
examples/cpp/topic_instances/PublisherApp.cpp Show resolved Hide resolved
examples/cpp/topic_instances/PublisherApp.cpp Outdated Show resolved Hide resolved
examples/cpp/topic_instances/PublisherApp.cpp Show resolved Hide resolved
examples/cpp/topic_instances/PublisherApp.cpp Outdated Show resolved Hide resolved
examples/cpp/topic_instances/PublisherApp.cpp Show resolved Hide resolved
examples/cpp/topic_instances/SubscriberApp.cpp Show resolved Hide resolved
examples/cpp/topic_instances/SubscriberApp.cpp Outdated Show resolved Hide resolved
examples/cpp/topic_instances/SubscriberApp.cpp Outdated Show resolved Hide resolved
@JesusPoderoso JesusPoderoso force-pushed the feature/example_refactor/topic_instances branch from f41ccdd to 6049e6a Compare July 15, 2024 09:03
@JesusPoderoso JesusPoderoso force-pushed the feature/example_refactor/topic_instances branch from b15865a to 815d97a Compare July 18, 2024 08:58
@elianalf elianalf self-requested a review July 18, 2024 09:03
@JesusPoderoso
Refs #21186: Implement topic instances example
38ae117 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Generate types
64896e0 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Add README.md
9d2c7d1 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Add example test
a798be4 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Remove previous example
aef7536 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Remove already merged old examples
f118534 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Update versions.md
c3a3019 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Please linters
08b7f91 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Fix windows build issues
935a561 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Reduce the default instances to 4 (looks better in Shape…
c528773 
…sDemo)

Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Fix shape size bounds, and set timeout as seconds
e20e84f 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Add instance methods: register and disposed
38e7a3b 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Please linter
e9f3729 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Update guards
80892a5 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Fix windows build
a7c2211 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Apply Miguel' suggestion
8431053 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Re-order methods
66b5414 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Apply minor suggestions
2ede928 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Allow introducing a set of colors as argument
7a17768 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Register instance on construction and move shape locatio…
0eb2739 
…n to publisher

Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Update README.md
27e348c 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Explicitly include array to avoid win build issues
b006cd4 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Fix corner case
c123cd7 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Fix cornercase properly
78bd1a6 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #21186: Apply rev suggestions (2)
4c5fa90 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso JesusPoderoso force-pushed the feature/example_refactor/topic_instances branch from 815d97a to 4c5fa90 Compare July 19, 2024 11:26
@JesusPoderoso JesusPoderoso requested review from elianalf and removed request for elianalf July 19, 2024 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elianalf elianalf Awaiting requested review from elianalf

Requested changes must be addressed to merge this pull request.

Assignees

@JesusPoderoso JesusPoderoso

Labels
ci-pending PR which CI is running
Projects
None yet
Milestone
v3.0.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@JesusPoderoso @elianalf