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

Process an empty YAML when a configuration file isn't provided #62

Merged
merged 6 commits into from
Jan 8, 2024

Conversation

Tempate
Copy link
Contributor

@Tempate Tempate commented Dec 14, 2023

In the previous version, when a user didn't provide a configuration file, the Fast-DDS Spy didn't process the default YAML configuration. To fix it, in this version when a configuration file isn't provided, the Fast-DDS Spy processes an empty YAML into which it loads the default YAML configuration.

Copy link

codecov bot commented Dec 14, 2023

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (774268b) 65.19% compared to head (d20f755) 64.96%.

Files Patch % Lines
...astddsspy_yaml/src/cpp/YamlReaderConfiguration.cpp 50.00% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
- Coverage   65.19%   64.96%   -0.23%     
==========================================
  Files          22       22              
  Lines        1109     1119      +10     
  Branches      389      395       +6     
==========================================
+ Hits          723      727       +4     
- Misses        137      141       +4     
- Partials      249      251       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@irenebm irenebm left a comment

Choose a reason for hiding this comment

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

Add a test for scenarios where a user hasn't provided a configuration file.

@Tempate Tempate force-pushed the bugfix/process_yaml_without_file branch from c8346de to cce52b9 Compare January 8, 2024 09:33
@@ -0,0 +1,45 @@
# Copyright 2023 Proyectos y Sistemas de Mantenimiento SL (eProsima).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2023 Proyectos y Sistemas de Mantenimiento SL (eProsima).
# Copyright 2024 Proyectos y Sistemas de Mantenimiento SL (eProsima).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's what happens when you take a year to review my PRs...

Copy link
Contributor

Choose a reason for hiding this comment

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

Cheers to the power of patience and precision! Like a fine wine, my PRs took time to mature, and the celebration is just as sweet. Here's to the art of meticulous review and the joy of progress!

Copy link
Contributor

@irenebm irenebm left a comment

Choose a reason for hiding this comment

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

LGTM

@Tempate Tempate merged commit 3b76649 into main Jan 8, 2024
18 of 20 checks passed
@Tempate Tempate deleted the bugfix/process_yaml_without_file branch January 8, 2024 14:14
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