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 support for repeated capabilities on methods that support independent channels #1589

Merged
merged 11 commits into from
Apr 21, 2021

Conversation

ni-erikcrank
Copy link
Contributor

@ni-erikcrank ni-erikcrank commented Apr 15, 2021

  • This contribution adheres to CONTRIBUTING.md.
  • [ ]I've updated CHANGELOG.md if applicable.
  • [ ]I've added tests applicable for this pull request

The repeated capabilities functionality of attributes and methods will be tested after independent channels session constructor is implemented. System tests will be added as part of this Azure DevOps work item.

What does this Pull Request accomplish?

Adds repeated capabilities to methods that support independent channels.

List issues fixed by this Pull Request below, if any.

What testing has been done?

Verified that DCPower system tests pass.

@nimi-bot
Copy link

Can one of the admins verify this patch?

@ni-erikcrank ni-erikcrank changed the title Add repeated capabilities Add support for repeated capabilities on methods that support independent channels Apr 15, 2021
@codecov
Copy link

codecov bot commented Apr 15, 2021

Codecov Report

Merging #1589 (c03243c) into master (f9979a7) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1589   +/-   ##
=======================================
  Coverage   91.68%   91.68%           
=======================================
  Files          20       20           
  Lines        3463     3463           
=======================================
  Hits         3175     3175           
  Misses        288      288           
Flag Coverage Δ
codegenunittests 88.27% <ø> (ø)
nifakeunittests 96.36% <ø> (ø)
nimodinstunittests 95.37% <ø> (ø)
nitclkunittests 95.61% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9979a7...c03243c. Read the comment docs.

@sbethur
Copy link
Contributor

sbethur commented Apr 15, 2021

ok to test

@sbethur
Copy link
Contributor

sbethur commented Apr 19, 2021

We need system tests to validate calling the updated entry points with rep caps. Please add a link or work item number in which that will be done.

@marcoskirsch
Copy link
Member

@ni-erikcrank please re-run codegen and we'll merge.

@marcoskirsch
Copy link
Member

We need system tests to validate calling the updated entry points with rep caps. Please add a link or work item number in which that will be done.

Why not in the same PR?

Copy link
Member

@marcoskirsch marcoskirsch left a comment

Choose a reason for hiding this comment

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

Awaiting system test changes

@ni-erikcrank
Copy link
Contributor Author

System tests would not succeed until we have a constructor that allows creation of sessions with independent channels. I didn't want to submit failing system tests.

@marcoskirsch
Copy link
Member

System tests would not succeed until we have a constructor that allows creation of sessions with independent channels. I didn't want to submit failing system tests.

I think they would, if you pass something equivalent to "all the channels in the session" such as "" or something else. Nevertheless, as long as you have the Issue to remind us to do it in a cleaner manner, that's fine with me.

@ni-erikcrank
Copy link
Contributor Author

Let me know if you are still awaiting changes/actions from me.

@sbethur
Copy link
Contributor

sbethur commented Apr 20, 2021

Let me know if you are still awaiting changes/actions from me.

There are merge conflicts you need to resolve.

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.

4 participants