-
Notifications
You must be signed in to change notification settings - Fork 98
RFSG examples default value fix and clean up #2130
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2130 +/- ##
==========================================
+ Coverage 81.72% 89.18% +7.45%
==========================================
Files 29 71 +42
Lines 4115 18960 +14845
==========================================
+ Hits 3363 16909 +13546
- Misses 752 2051 +1299
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this matches other examples?
@marcoskirsch The option string default now aligns with other driver examples. However I do not see other drivers making resource name as required input. I made it required here mainly to catch it if its missing, at the example level itself instead of letting driver initialize error out. |
I think you should stay consistent. If you think making the resource name required, then make the change consistently in another PR. But I think many drivers allow leaving resource name blank in cases of simulation so that's probably why it's not required. |
Made the resource name to be not required to be consistent with other driver examples. Made the resource name default value to be PXI1Slot2 to be consistent with other driver examples. |
I've updated CHANGELOG.md if applicable.I've added tests applicable for this pull requestWhat does this Pull Request accomplish?
List issues fixed by this Pull Request below, if any.
None
What testing has been done?