Skip to content

Names -> Keywords batch 10 #6973

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

Conversation

WhiteBlackGoose
Copy link
Contributor

Replacing full type names with keywords when appropriate

Files affected: operations1.cs, parse2.cs, parse3.cs, tryparseex.cs, bcopy.cs, getbyte.cs, setbyte.cs, systembyte.cs, tostring.cs, charstructure.cs.

See #6920

Replacing full type names with keywords when appropriate

Files affected: operations1.cs, parse2.cs, parse3.cs, tryparseex.cs, bcopy.cs, getbyte.cs, setbyte.cs, systembyte.cs, tostring.cs, charstructure.cs.

See dotnet#6920
@WhiteBlackGoose WhiteBlackGoose requested a review from a team as a code owner August 4, 2021 18:56
@ghost ghost added the area-System.Runtime label Aug 4, 2021
@opbld33
Copy link

opbld33 commented Aug 4, 2021

Docs Build status updates of commit c9bad0e:

✅ Validation status: passed

File Status Preview URL Details
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.BlockCopy/CS/bcopy.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.Bytes/CS/getbyte.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.Bytes/CS/setbyte.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Byte Examples/CS/systembyte.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Byte.ToString/CS/tostring.cs ✅Succeeded
samples/snippets/csharp/VS_Snippets_CLR_System/system.Char [Type Level]/CS/charstructure.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.structure/cs/operations1.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.structure/cs/parse2.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.structure/cs/parse3.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.tryparse/cs/tryparseex.cs ✅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:

@BillWagner
Copy link
Member

closing and reopening to trigger the samples CI build

@BillWagner BillWagner closed this Aug 5, 2021
@BillWagner BillWagner reopened this Aug 5, 2021
@WhiteBlackGoose
Copy link
Contributor Author

@BillWagner I just want to 100% verify. Is it really the way you want me to do this? Is it convenient for you or other reviewers? Because there are 150 more branches to merge, those were only 20 or about that.

@opbld31
Copy link

opbld31 commented Aug 5, 2021

Docs Build status updates of commit c9bad0e:

✅ Validation status: passed

File Status Preview URL Details
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.BlockCopy/CS/bcopy.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.Bytes/CS/getbyte.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.Bytes/CS/setbyte.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Byte Examples/CS/systembyte.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Byte.ToString/CS/tostring.cs ✅Succeeded
samples/snippets/csharp/VS_Snippets_CLR_System/system.Char [Type Level]/CS/charstructure.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.structure/cs/operations1.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.structure/cs/parse2.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.structure/cs/parse3.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.tryparse/cs/tryparseex.cs ✅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:

@BillWagner
Copy link
Member

Tagging @adegeo

The snippets 5000 CI check failed because the nupkg isn't installed in the dependencies folder. Can you open a new PR since that was missed in #6982?

@BillWagner
Copy link
Member

BillWagner commented Aug 5, 2021

@WhiteBlackGoose Thanks for reaching out. Here's a detailed update:

Yes, we do like these changes. Making them is also driving us to add a check that we've had in the docs repo for some time now: a CI build on all sample code. Briefly, the snippets 5000 checks any PR that changes source code to ensure the code changed is included in a project, and if so, it builds that project.

For historical reasons, the sample code in this repo typically didn't have a project file. It was migrated from an internal system that did verification builds. Our expectation is that these changes will fail in the snippets 5000 build because most of these folders won't have a project file.

The fix is reasonably simple: add a project file for a console app. Right now, the project file can use .NET Core 3.1 (latest LTS), .NET 5 (latest release) or .NET 6 (preview). Our version checker will flag any version that's out of support on a monthly basis.

One challenge we've had in some folders is where there are multiple Main methods in different files. These are quite common, because we have many examples that are full programs. Those are used to support C# interactive, where we run many of the snippets in the browser on docs. To make a project file that passes the CI build, specify a startup object, and add a new Main method that calls the existing Main method(s) in turn. You can see an example here: https://github.com/dotnet/docs/tree/main/docs/csharp/language-reference/builtin-types/snippets/shared

Let us know how that works, if you are are comfortable doing these extra steps on these PRs. It will really help us validate the quality of these samples.

I'll wait to review the other PRs opened recently while we concentrate on this first PR.

@WhiteBlackGoose
Copy link
Contributor Author

@BillWagner so, I have to add a project file to each folder, right? Then here's a question. Why not to add it to each file, instead of each folder? Iirc all snippets are runnable independently, hence we can do it.

@adegeo
Copy link
Contributor

adegeo commented Aug 5, 2021

so, I have to add a project file to each folder, right? Then here's a question. Why not to add it to each file, instead of each folder? Iirc all snippets are runnable independently, hence we can do it.

Currently the build system wants a single project per folder. To do multiple projects you would have to move the files to sub folders and add projects to each folder. Then you would have to update the articles that reference the sample, it's a bit more work.

Adding the project file is generally simple, as Bill pointed out. However, with multiple main objects, you may actually need to specify the <StartupObject>class method</StartupObject> to indicate which main is the main object. The snippet system doesn't actually RUN the code, it just checks for compile. So (@BillWagner) you don't need to make a Main that calls the other Mains, it will be pointless. You just need the compiler to pass.

@BillWagner
Copy link
Member

you don't need to make a Main that calls the other Mains, it will be pointless. You just need the compiler to pass.

That's true. I do find that I like being able to run the samples locally. That's why I've added that.

@BillWagner
Copy link
Member

triggering a fresh build.

@BillWagner BillWagner closed this Aug 5, 2021
@BillWagner BillWagner reopened this Aug 5, 2021
@BillWagner
Copy link
Member

@WhiteBlackGoose The last snippets 5000 build shows the errors we'd expect from missing project files.

@opbld31
Copy link

opbld31 commented Aug 5, 2021

Docs Build status updates of commit c9bad0e:

✅ Validation status: passed

File Status Preview URL Details
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.BlockCopy/CS/bcopy.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.Bytes/CS/getbyte.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.Bytes/CS/setbyte.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Byte Examples/CS/systembyte.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Byte.ToString/CS/tostring.cs ✅Succeeded
samples/snippets/csharp/VS_Snippets_CLR_System/system.Char [Type Level]/CS/charstructure.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.structure/cs/operations1.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.structure/cs/parse2.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.structure/cs/parse3.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.tryparse/cs/tryparseex.cs ✅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:

Copy link

@pgovind pgovind left a comment

Choose a reason for hiding this comment

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

Code changes look fine. Will leave the CI/build issues to the docs folks

@WhiteBlackGoose
Copy link
Contributor Author

@pgovind @adamsitnik I just didn't have time yet to add the necessary files, as per Bill Wagner's comment. Sorry, should've converted the PR to draft

@WhiteBlackGoose WhiteBlackGoose marked this pull request as draft August 9, 2021 18:39
@opbld30
Copy link

opbld30 commented Aug 12, 2021

Docs Build status updates of commit f7a60f0:

✅ Validation status: passed

File Status Preview URL Details
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.BlockCopy/CS/QuackTest.csproj ✅Succeeded
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.BlockCopy/CS/bcopy.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.Bytes/CS/getbyte.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.Bytes/CS/setbyte.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Byte Examples/CS/systembyte.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Byte.ToString/CS/tostring.cs ✅Succeeded
samples/snippets/csharp/VS_Snippets_CLR_System/system.Char [Type Level]/CS/charstructure.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.structure/cs/operations1.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.structure/cs/parse2.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.structure/cs/parse3.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.tryparse/cs/tryparseex.cs ✅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:

@BillWagner
Copy link
Member

closing and reopening for a fresh build.

@BillWagner BillWagner closed this Dec 10, 2021
@BillWagner BillWagner reopened this Dec 10, 2021
@opbld34
Copy link

opbld34 commented Dec 10, 2021

Docs Build status updates of commit f7a60f0:

✅ Validation status: passed

File Status Preview URL Details
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.BlockCopy/CS/QuackTest.csproj ✅Succeeded
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.BlockCopy/CS/bcopy.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.Bytes/CS/getbyte.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Buffer.Bytes/CS/setbyte.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Byte Examples/CS/systembyte.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.Byte.ToString/CS/tostring.cs ✅Succeeded
samples/snippets/csharp/VS_Snippets_CLR_System/system.Char [Type Level]/CS/charstructure.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.structure/cs/operations1.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.structure/cs/parse2.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.structure/cs/parse3.cs ✅Succeeded View
samples/snippets/csharp/VS_Snippets_CLR_System/system.boolean.tryparse/cs/tryparseex.cs ✅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:

@gewarren
Copy link
Contributor

gewarren commented Apr 1, 2022

I'm going to close this PR since the snippets have all moved to new folders. Feel free to continue this work in the new snippet folder structure. Also, most snippets should have associated project files now, so you won't need to add too many of those.

@gewarren gewarren closed this Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants