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.

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

Add configuration by profile to AdvanceConfigurationExample [18694] #3554

Merged
merged 6 commits into from
Jun 14, 2023

Conversation

jparisu
Copy link
Contributor

@jparisu jparisu commented May 31, 2023

Add a new argument to AdvanceConfigurationExample that allows to configure a DomainParticipant from XML.

Description

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • [N/A] 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
  • [N/A] Any new/modified methods have been properly documented using Doxygen.
  • Changes are ABI compatible.
  • Changes are API compatible.
  • [N/A] 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.
  • Check contributor checklist is correct.
  • [N/A] Check CI results: changes do not issue any warning.
  • [N/A] Check CI results: failing tests are unrelated with the changes.

@jparisu jparisu added the skip-ci Automatically pass CI label May 31, 2023
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

  1. I think some changes in your local repository have made it to this PR. The commit and PR title states that the goal is to introduce XML configuration support in the AdvanceConfigurationExample, but the SecureHelloWorldExample and the HelloWorldExample are also modified and I think that was not your intention.
  2. Please, fix the uncrustify linter which is complaining in the AdvanceConfigurationExample main file.
  3. README has not been updated with this new option.
  4. Being an example, and thus oriented for user understanding, maybe we should add an XML configuration file so the user can check this functionality.
  5. Please, fill contributor checklist (marking as N/A those checks not applicable).

@JLBuenoLopez JLBuenoLopez added this to the v2.11.0 milestone Jun 5, 2023
@JLBuenoLopez JLBuenoLopez added no-test Skip CI tests if PR marked with this label and removed skip-ci Automatically pass CI labels Jun 5, 2023
@jparisu jparisu force-pushed the feature/advance-example-xml branch from cfe2a82 to 7eed7f6 Compare June 13, 2023 13:37
@jparisu
Copy link
Contributor Author

jparisu commented Jun 13, 2023

  1. Fixed in 626df5a
  2. 7eed7f6
  3. 1b36e04
  4. 1b36e04
  5. 👍

@jparisu jparisu changed the title Add configuration by profile to AdvanceConfigurationExample Add configuration by profile to AdvanceConfigurationExample [18694] Jun 13, 2023
@JesusPoderoso
Copy link
Contributor

@richiprosima please test this

Signed-off-by: jparisu <[email protected]>
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

LGTM

@JLBuenoLopez
Copy link
Contributor

@richiprosima please test this

@JLBuenoLopez JLBuenoLopez added the ci-pending PR which CI is running label Jun 13, 2023
@JLBuenoLopez JLBuenoLopez merged commit 339f9ef into master Jun 14, 2023
@JLBuenoLopez JLBuenoLopez deleted the feature/advance-example-xml branch June 14, 2023 12:21
roscan-tech pushed a commit to roscan-tech/Fast-DDS that referenced this pull request Sep 8, 2024
…Prosima#3554)

* Add configuration by profile to AdvanceConfigurationExample

Signed-off-by: jparisu <[email protected]>

* fix args inconsistency

Signed-off-by: jparisu <[email protected]>

* Restore old files uploaded by mistake

Signed-off-by: jparisu <[email protected]>

* Add conf example file

Signed-off-by: jparisu <[email protected]>

* uncrustify

Signed-off-by: jparisu <[email protected]>

* apply suggestions

Signed-off-by: jparisu <[email protected]>

---------

Signed-off-by: jparisu <[email protected]>
Signed-off-by: roscan-tech <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-pending PR which CI is running no-test Skip CI tests if PR marked with this label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants