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.

Sign up for GitHub

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

Jump to bottom

Change Option<TType>.Text to string? #2063

Merged
merged 6 commits into from
May 16, 2024
Merged

Change Option<TType>.Text to string? #2063

merged 6 commits into from
May 16, 2024

Conversation

StevenTCramer
Copy link
Contributor

Pull Request

📖 Description

See linked Discussion for more details.

This PR includes the following changes:

  1. Type Update:

    • Modified the type from [TType?] to [string?]

  2. Demos Update:

    • Updated all relevant demos to reflect the type change.

🎫 Issues

#2062

👩‍💻 Reviewer Notes

📑 Test Plan

There were no tests effected. My manual testing was to run the demo and insure it still worked. I ran dotnet test.

✅ Checklist

General

  • I have added tests for my changes. (There were no tests other than the demo for this.)
  • I have tested my changes. (All existing tests passed)
  • I have updated the project documentation to reflect my changes. (None found for this class)
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

  • I have modified a class used by an existing component
  • I have validated the Unit Tests for an existing component

⏭ Next Steps

@StevenTCramer
In Option<TType> change the type of Text to string? If it is TType …
bfb8800 
…it only adds value for TType of string otherwise it makes no sense if Text is some Ojbect.
@StevenTCramer
Update the demo apps to use Text as a string.
c490b54
@vnbaaij vnbaaij enabled auto-merge (squash) May 16, 2024 15:30
@vnbaaij vnbaaij merged commit a43b7c3 into microsoft:dev May 16, 2024
2 checks passed
vnbaaij added a commit that referenced this pull request May 17, 2024
@StevenTCramer @vnbaaij
[Lists] Change Option<TType>.Text to string? (#2063)
3a5018e 
* In Option<TType> change the type of `Text` to string?  If it is TType it only adds value for TType of string otherwise it makes no sense if Text is some Ojbect.

* Update the demo apps to use Text as a string.

* Use string literal instead of ToString.

* Now we can remove redundant .ToString

---------

Co-authored-by: Vincent Baaij <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vnbaaij vnbaaij vnbaaij approved these changes

@dvoituron dvoituron Awaiting requested review from dvoituron dvoituron is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@StevenTCramer @vnbaaij