Skip to content

Conversation

@pkulikov
Copy link
Contributor

@pkulikov pkulikov commented Apr 5, 2018

Combined C# Concepts classes article into the Classes (C# Programming guide) article.

Additionally:

  • Inlined small code pieces as it makes a source Markdown file more readable.
  • Minor editions.

Fixes #964

In tandem with PR dotnet/samples#8

@pkulikov pkulikov requested a review from BillWagner as a code owner April 5, 2018 20:42
@pkulikov pkulikov changed the title Remove classes duplication Remove duplicate classes topic from C# guide Apr 5, 2018
@pkulikov pkulikov changed the title Remove duplicate classes topic from C# guide WIP Remove duplicate classes topic from C# guide Apr 6, 2018
@pkulikov
Copy link
Contributor Author

pkulikov commented Apr 6, 2018

@BillWagner I put WIP as I also want to move the example snippet to the samples repo.

@mairaw
Copy link
Contributor

mairaw commented Apr 6, 2018

@pkulikov so we have a current work item (#1255) that we haven't been able to finish yet (it was supposed to be automated) but I manually change some PRs when I see they're touching snippets in these codesnippet folders.

The first time we migrated these topics, they erroneously broken down each sample into these tiny little snippets. But we do have the snippets in the samples repo already and you can see how the topic should have looked like if it got migrated correctly the first time here:
https://raw.githubusercontent.com/dotnet/docs/migration_csharpvb_reexport/docs/csharp/programming-guide/classes-and-structs/classes.md

So the original snippet for this probably lives here:
https://github.com/dotnet/samples/blob/master/snippets/csharp/VS_Snippets_VBCSharp/csProgGuideObjects/CS/Objects.cs

But you can probably change the samples location to ~/samples now that they don't live in this repo anymore. Something to test.

@pkulikov
Copy link
Contributor Author

pkulikov commented Apr 6, 2018

@mairaw interesting that that file with a lot of snippets doesn't contain snippets for the topic in interest; as if those were cut out of the file.

I've made the PR in the samples repo to move the snippet to programming guide snippets here:
https://github.com/dotnet/samples/tree/master/snippets/csharp/programming-guide/classes-and-structs

I assume the old way to link snippets from the Markdown files continues to work even when snippets are in the different repo.

This PR is ready for review.

@pkulikov pkulikov changed the title WIP Remove duplicate classes topic from C# guide Remove duplicate classes topic from C# guide Apr 6, 2018
@pkulikov
Copy link
Contributor Author

pkulikov commented Apr 6, 2018

@mairaw I've just realized that, of course, build fails - I should wait for PR dotnet/samples#8 in the samples repo to be merged in the repo. However, that would break current master here. How do I go from here? (Option: update PR in the samples repo not to break master here.)

@mairaw
Copy link
Contributor

mairaw commented Apr 6, 2018

It's something we also need to understand better ourselves. It's been in my to-do list to talk to the engineering team about this workflow. When I catch a break, I'll take a look at your changes here and there. Exciting times! 😄

@pkulikov
Copy link
Contributor Author

pkulikov commented Apr 6, 2018

We need a notion of tandem PRs :)

@pkulikov
Copy link
Contributor Author

pkulikov commented Apr 9, 2018

@BillWagner there was the merge conflict with master; I've resolved it with the latest commit

@pkulikov
Copy link
Contributor Author

pkulikov commented Apr 9, 2018

@BillWagner and we should merge this PR as fast as possible: all other PRs do not build because of this one.

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.

@pkulikov Overall, this is a great improvement to combine these two topics. There were a couple sentences that could be misinterpreted. I offered suggestions for other wording. Once that's done, I'll :shipit:

Thanks again.

A *class* is a construct that enables you to create your own custom types by grouping together variables of other types, methods and events. A class is like a blueprint. It defines the data and behavior of a type. If the class is not declared as static, client code can use it by creating *objects* or *instances* which are assigned to a variable. The variable remains in memory until all references to it go out of scope. At that time, the CLR marks it as eligible for garbage collection. If the class is declared as [static](../../../csharp/language-reference/keywords/static.md), then only one copy exists in memory and client code can only access it through the class itself, not an *instance variable*. For more information, see [Static Classes and Static Class Members](../../../csharp/programming-guide/classes-and-structs/static-classes-and-static-class-members.md).

Unlike structs, classes support *inheritance*, a fundamental characteristic of object-oriented programming. For more information, see [Inheritance](../../../csharp/programming-guide/classes-and-structs/inheritance.md).
A *class* is a construct that enables you to create your own custom types by grouping together variables of other types, methods and events. A class is like a blueprint. It defines the data and behavior of a type. If the class is not declared as static, client code can create *instances* of a class or *objects* which are assigned to a variable. The instance of a class remains in memory until all references to it go out of scope. At that time, the CLR marks it as eligible for garbage collection. If the class is declared as [static](../../../csharp/language-reference/keywords/static.md), then only one copy exists in memory and client code can only access it through the class itself, not an *instance variable*. For more information, see [Static Classes and Static Class Members](../../../csharp/programming-guide/classes-and-structs/static-classes-and-static-class-members.md).
Copy link
Member

Choose a reason for hiding this comment

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

There are a few sentences here that could be more clear:

If the class is not declared as static, client code can create instances of a class or objects which are assigned to a variable.

Consider: "If the class is not declared as static, client code can create instances of it. These instances are objects which are assigned to a variable."

If the class is declared as static, then only one copy exists in memory and client code can only access it through the class itself, not an instance variable.

Consider: "If the class is declared as static, you cannot create instances, and client code can only access it through the class itself."

@pkulikov
Copy link
Contributor Author

pkulikov commented Apr 9, 2018

@BillWagner thank you for feedback, I've updated wording as you've suggested.

@mairaw
Copy link
Contributor

mairaw commented Apr 9, 2018

If you approved this @BillWagner, I'll merge it then because all builds are breaking now because the samples change is in but not this one.

@mairaw mairaw merged commit ea49a31 into dotnet:master Apr 9, 2018
@pkulikov pkulikov deleted the remove-classes-duplication branch April 9, 2018 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants