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

Move System.Diagnostics.Process lengthy remarks to files #5360

Closed
wants to merge 9 commits into from
Closed

Move System.Diagnostics.Process lengthy remarks to files #5360

wants to merge 9 commits into from

Conversation

carlossanlop
Copy link
Member

@carlossanlop carlossanlop commented Feb 22, 2021 •
edited
Loading

Based on @gewarren suggestion here, I am moving all the lengthy remarks from Process.xml to their own md files.

I organized the md files the same way we have the API files organized in the xml folder (seemed much more organized than the way we have our snippets). Remarks that belong to different overloads of the same method are added with the type of their arguments.

This PR is blocking dotnet/runtime#48137

@carlossanlop carlossanlop self-assigned this Feb 22, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-System.Security Issues related to security practices for .NET developers. label Feb 22, 2021
@carlossanlop carlossanlop mentioned this pull request Feb 22, 2021
Closed
gewarren
gewarren reviewed Feb 23, 2021
View reviewed changes
Copy link
Contributor

@gewarren gewarren left a comment

Choose a reason for hiding this comment

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

Overall looks great. Mainly one question about the file naming.

@gewarren gewarren closed this Mar 3, 2021
@gewarren gewarren reopened this Mar 3, 2021
@opbld30
Copy link

opbld30 commented Mar 3, 2021

Docs Build status updates of commit d61d5d9:

✅ Validation status: passed

File Status Preview URL Details
includes/remarks/System.Diagnostics/DataReceivedEventArgs/Data.md ✅Succeeded View
includes/remarks/System.Diagnostics/DataReceivedEventArgs/DataReceivedEventArgs.md ✅Succeeded View
includes/remarks/System.Diagnostics/DataReceivedEventHandler/DataReceivedEventHandler.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/.ctor.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/BasePriority.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/BeginErrorReadLine.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/BeginOutputReadLine.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/CancelErrorRead.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/CancelOutputRead.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/EnableRaisingEvents.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/ErrorDataReceived.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/ExitCode.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/Exited.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/GetProcessById_Int32.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/GetProcessById_Int32_String.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/GetProcesses.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/GetProcessesByName_String.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/GetProcessesByName_String_String.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/GetProcesses_String.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/HasExited.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/Id.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/Kill.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/OutputDataReceived.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/PriorityClass.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/Process.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/ProcessorAfinity.md ✅Succeeded
includes/remarks/System.Diagnostics/Process/StandardError.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/StandardOutput.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/Start.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/StartInfo.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/Start_ProcessStartInfo.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/Start_String.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/Start_String_String.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/Start_String_String_SecureString_String.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/Start_String_String_String_SecureString_String.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/SynchronizingObject.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/WaitForExit.md ✅Succeeded View
includes/remarks/System.Diagnostics/Process/WaitForExit_Int32.md ✅Succeeded View
xml/System.Diagnostics/DataReceivedEventArgs.xml ✅Succeeded View
xml/System.Diagnostics/DataReceivedEventHandler.xml ✅Succeeded View
xml/System.Diagnostics/Process.xml ✅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:

@opbld32
Copy link

opbld32 commented Mar 4, 2021

Docs Build status updates of commit 61d546c:

❌ Validation status: errors

Please follow instructions here which may help to resolve issue.

File Status Preview URL Details
xml/System.Diagnostics/Process.xml ❌Error Details

xml/System.Diagnostics/Process.xml

  • Line 0, Column 0: [Error-include-not-found] Invalid include link: '~/includes/remarks/System.Diagnostics/Process/ProcessAfinity.md'.

For more details, please refer to the build report.

If you see build warnings/errors with permission issues, it might be due to single sign-on (SSO) enabled on Microsoft's GitHub organizations. Please follow instructions here to re-authorize your GitHub account to Docs Build.

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.

Note: Your PR may contain errors or warnings unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

@opbld30
Copy link

opbld30 commented Mar 4, 2021

Docs Build status updates of commit 392d7e6:

❌ Validation status: errors

Please follow instructions here which may help to resolve issue.

File Status Preview URL Details
xml/System.Diagnostics/Process.xml ❌Error Details

xml/System.Diagnostics/Process.xml

  • Line 0, Column 0: [Error-include-not-found] Invalid include link: '~/includes/remarks/System.Diagnostics/Process/ProcessorAffinity.md'.

For more details, please refer to the build report.

If you see build warnings/errors with permission issues, it might be due to single sign-on (SSO) enabled on Microsoft's GitHub organizations. Please follow instructions here to re-authorize your GitHub account to Docs Build.

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.

Note: Your PR may contain errors or warnings unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

adamsitnik
adamsitnik approved these changes Mar 4, 2021
View reviewed changes
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 !

includes/remarks/System.Diagnostics/Process/GetProcessesByName_String.md
[!code-cpp[Process.GetProcesses_noexception#1](~/samples/snippets/cpp/VS_Snippets_CLR/Process.GetProcesses_noexception/CPP/processstaticget.cpp#1)]
[!code-csharp[Process.GetProcesses_noexception#1](~/samples/snippets/csharp/VS_Snippets_CLR/Process.GetProcesses_noexception/CS/processstaticget.cs#1)]
[!code-vb[Process.GetProcesses_noexception#1](~/samples/snippets/visualbasic/VS_Snippets_CLR/Process.GetProcesses_noexception/VB/processstaticget.vb#1)]

Copy link
Member

Choose a reason for hiding this comment

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

nit: empty line

Suggested change

includes/remarks/System.Diagnostics/Process/GetProcessesByName_String_String.md
[!code-cpp[Process.GetProcesses_noexception#1](~/samples/snippets/cpp/VS_Snippets_CLR/Process.GetProcesses_noexception/CPP/processstaticget.cpp#1)]
[!code-csharp[Process.GetProcesses_noexception#1](~/samples/snippets/csharp/VS_Snippets_CLR/Process.GetProcesses_noexception/CS/processstaticget.cs#1)]
[!code-vb[Process.GetProcesses_noexception#1](~/samples/snippets/visualbasic/VS_Snippets_CLR/Process.GetProcesses_noexception/VB/processstaticget.vb#1)]

Copy link
Member

Choose a reason for hiding this comment

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

nit: empty line

Suggested change

includes/remarks/System.Diagnostics/Process/StandardError.md
[!code-cpp[Process_StandardError#1](~/samples/snippets/cpp/VS_Snippets_CLR/Process_StandardError/CPP/source.cpp#1)]
[!code-csharp[Process_StandardError#1](~/samples/snippets/csharp/VS_Snippets_CLR/Process_StandardError/CS/source.cs#1)]
[!code-vb[Process_StandardError#1](~/samples/snippets/visualbasic/VS_Snippets_CLR/Process_StandardError/VB/source.vb#1)]

Copy link
Member

Choose a reason for hiding this comment

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

nit: empty line

Suggested change

includes/remarks/System.Diagnostics/Process/StandardOutput.md
[!code-cpp[Process_StandardOutput#2](~/samples/snippets/cpp/VS_Snippets_CLR/Process_StandardOutput/CPP/process_standardoutput.cpp)]
[!code-csharp[Process_StandardOutput#2](~/samples/snippets/csharp/VS_Snippets_CLR/Process_StandardOutput/CS/process_standardoutput.cs)]
[!code-vb[Process_StandardOutput#2](~/samples/snippets/visualbasic/VS_Snippets_CLR/Process_StandardOutput/VB/process_standardoutput.vb)]

Copy link
Member

Choose a reason for hiding this comment

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

nit: empty line

Suggested change

includes/remarks/System.Diagnostics/Process/StartInfo.md
[!code-cpp[Process.Start_instance#1](~/samples/snippets/cpp/VS_Snippets_CLR/Process.Start_instance/CPP/processstart.cpp#1)]
[!code-csharp[Process.Start_instance#1](~/samples/snippets/csharp/VS_Snippets_CLR/Process.Start_instance/CS/processstart.cs#1)]
[!code-vb[Process.Start_instance#1](~/samples/snippets/visualbasic/VS_Snippets_CLR/Process.Start_instance/VB/processstart.vb#1)]

Copy link
Member

Choose a reason for hiding this comment

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

nit: empty line

Suggested change

includes/remarks/System.Diagnostics/Process/Start_String_String_SecureString_String.md

[!code-csharp[System.Diagnostics.Process.Start#1](~/samples/snippets/csharp/VS_Snippets_CLR_System/system.Diagnostics.Process.Start/CS/program.cs#1)]
[!code-vb[System.Diagnostics.Process.Start#1](~/samples/snippets/visualbasic/VS_Snippets_CLR_System/system.Diagnostics.Process.Start/VB/program.vb#1)]

Copy link
Member

Choose a reason for hiding this comment

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

nit: empty line

Suggested change

includes/remarks/System.Diagnostics/Process/SynchronizingObject.md
[!code-cpp[Process_SynchronizingObject#1](~/samples/snippets/cpp/VS_Snippets_CLR/Process_SynchronizingObject/CPP/process_synchronizingobject.cpp#1)]
[!code-csharp[Process_SynchronizingObject#1](~/samples/snippets/csharp/VS_Snippets_CLR/Process_SynchronizingObject/CS/process_synchronizingobject.cs#1)]
[!code-vb[Process_SynchronizingObject#1](~/samples/snippets/visualbasic/VS_Snippets_CLR/Process_SynchronizingObject/VB/process_synchronizingobject.vb#1)]

Copy link
Member

Choose a reason for hiding this comment

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

nit: empty line

Suggested change

xml/System.Diagnostics/DataReceivedEventArgs.xml
Comment on lines +37 to +41
<format type="text/markdown"><![CDATA[

[!INCLUDE[remarks](~/includes/remarks/System.Diagnostics/DataReceivedEventArgs/DataReceivedEventArgs.md)]

]]></format>
Copy link
Member

Choose a reason for hiding this comment

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

are the extra lines needed?

Suggested change
<format type="text/markdown"><![CDATA[
[!INCLUDE[remarks](~/includes/remarks/System.Diagnostics/DataReceivedEventArgs/DataReceivedEventArgs.md)]
]]></format>
<format type="text/markdown"><![CDATA[[!INCLUDE[remarks](~/includes/remarks/System.Diagnostics/DataReceivedEventArgs/DataReceivedEventArgs.md)]]]></format>

Base automatically changed from master to main March 5, 2021 20:52
@maryamariyan maryamariyan added area-System.Diagnostics and removed area-System.Security Issues related to security practices for .NET developers. labels Mar 12, 2021
@gewarren
Copy link
Contributor

@carlossanlop Should we close this PR?

@carlossanlop
Copy link
Member Author

This area has had many changes. I'll do the process again, but will make sure it can all be done automatically via DocsPortingTool.

@carlossanlop carlossanlop deleted the ProcessRemarks branch August 18, 2021 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gewarren gewarren gewarren left review comments

@adamsitnik adamsitnik adamsitnik approved these changes

@jozkee jozkee Awaiting requested review from jozkee

@safern safern Awaiting requested review from safern

Assignees

@carlossanlop carlossanlop

Labels
area-System.Diagnostics
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@carlossanlop @opbld30 @opbld32 @gewarren @adamsitnik @maryamariyan