Skip to content

Conversation

@carlossanlop
Copy link
Contributor

Like with previous PRs, I am trying to improve our System.IO examples and make sure we only test the relevant code.

I am removing comments too, I felt they were redundant, but if you think a comment would be helpful, let me know.

I ensured they all compiled and executed successfully.

@carlossanlop carlossanlop added the doc-enhancement Improve the current content label Feb 5, 2021
@carlossanlop carlossanlop requested review from a team, adamsitnik, gewarren and jozkee February 5, 2021 06:14
@carlossanlop carlossanlop self-assigned this Feb 5, 2021
@ghost ghost added the area-System.Security Issues related to security practices for .NET developers. label Feb 5, 2021
@opbld32
Copy link

opbld32 commented Feb 5, 2021

Docs Build status updates of commit ed157b8:

✅ Validation status: passed

File Status Preview URL Details
samples/snippets/cpp/VS_Snippets_CLR_Classic/classic NotifyFilters Example/CPP/source.cpp ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_Classic/classic NotifyFilters Example/CS/source.cs ✅Succeeded View
samples/snippets/visualbasic/VS_Snippets_CLR_Classic/classic NotifyFilters Example/VB/source.vb ✅Succeeded View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

Console.WriteLine($"File: {e.FullPath} {e.ChangeType}");
private static void OnChanged(object sender, FileSystemEventArgs e)
{
if (e.ChangeType != WatcherChangeTypes.Changed)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I discovered that the OnChanged event method gets called on any change type. The only one I care about in my example is Changed.

Copy link

Choose a reason for hiding this comment

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

Was this with .NET Core / .NET 5? I don't think this should be possible with .NET 5.

The Windows, Linux and Mac implementations all raise the Changed event with NotifyFileSystemEventArgs():

https://github.com/dotnet/runtime/blob/385b6f3e5fd6fbf08a8a82bde5a5f57f18018c2a/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.cs#L429-L469

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @carlossanlop !

@opbld31
Copy link

opbld31 commented Feb 18, 2021

Docs Build status updates of commit 66494b1:

✅ Validation status: passed

File Status Preview URL Details
samples/snippets/cpp/VS_Snippets_CLR_Classic/classic NotifyFilters Example/CPP/source.cpp ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_Classic/classic NotifyFilters Example/CS/source.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_Classic/classic NotifyFilters Example/CS/source.exe ✅Succeeded
samples/snippets/visualbasic/VS_Snippets_CLR_Classic/classic NotifyFilters Example/VB/source.vb ✅Succeeded View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

@carlossanlop
Copy link
Contributor Author

Looks good now in the preview.

@carlossanlop carlossanlop merged commit 9878fe4 into dotnet:master Feb 18, 2021
@carlossanlop carlossanlop deleted the FSW branch February 18, 2021 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Security Issues related to security practices for .NET developers. doc-enhancement Improve the current content

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants