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

Evalg 81 - Adding content filtering C# #702

Merged
merged 11 commits into from
Sep 24, 2024

Conversation

xFrenchy
Copy link
Contributor

Summary

EVALG-81

Details and comments

Adding content filtering C# to tutorial

Checks

  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.

Questions:

  • For the Optional exercise of updating the filter, I added a third command line argument. (pub, sub, filter). Is this a good assumption?
  • We state that the filter can now be changed at runtime, but the example doesn't truly show that. The filter gets changed to "Kitchen" and is only changed once, artificially. Should we update it so that the user gets to decide when a change happens and to what
  • When creating the parameters, in C++11 you used a StringSeq. In C# it seems that I should have used a Rti.Types.Sequence but that is equivalent to a native List. So I used a List, is that alright?
  • In the README, I state how to run the example, however in C++ README, I see that it simply says to see the tutorial for instructions. The tutorial doesn't have C# yet so I wasn't sure if I should repeat the same instructions (although it would be missing) or to add them to the README for now and update later?

@alexcamposruiz alexcamposruiz changed the base branch from develop to master August 26, 2024 16:27
alexcamposruiz
alexcamposruiz previously approved these changes Sep 24, 2024
@alexcamposruiz alexcamposruiz merged commit 1dc2cfa into rticommunity:master Sep 24, 2024
1 check passed
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