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

Update Path.GetDirectoryName Snippet3 #10853

Merged
merged 1 commit into from
Jan 21, 2025
Merged

Conversation

DickBaker
Copy link
Contributor

Summary

the current docs are outdated to NRT and confuse quotes

  1. variables type should be string**?** not string
  2. uses old-style format string (new would use interpolated syntax)
  3. filepath reassignment should use conditional syntax
  4. results suggest using single-quote [i.e. char not string], ugh
  5. the final returned result is null but demo output shows '' which is factually and visually correct but confusingly indistinguishable from "" emptystring

Describe your changes here.

I propose this replacement C# code to improve this Snippet3 accuracy and clarity

the current docs are outdated to NRT and confuse quotes
1. variable should be type string? not string
2. uses old-style format string (new would use interpolated syntax)
3. filepath reassignment should use conditional syntax
4. results suggest using single-quote [i.e. char not string], ugh
5. the final returned result is null but demo output shows '' which is factually visually correct but confusingly indistinuishable from "" emptystring

I propose replacement C# code for this Snippet3
@DickBaker DickBaker requested a review from a team as a code owner January 20, 2025 19:43
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jan 20, 2025
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-io

Copy link

Learn Build status updates of commit 1bc1f95:

✅ Validation status: passed

File Status Preview URL Details
snippets/csharp/System.IO/Path/ChangeExtension/pathmembers.cs ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

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

This LGTM @DickBaker

I'll :shipit: now.

@BillWagner BillWagner merged commit 564260b into dotnet:main Jan 21, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.IO community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants