Skip to content

Replaced IComparer example#245

Merged
rpetrusha merged 4 commits intodotnet:masterfrom
rpetrusha:icomparer
Jun 6, 2018
Merged

Replaced IComparer example#245
rpetrusha merged 4 commits intodotnet:masterfrom
rpetrusha:icomparer

Conversation

@rpetrusha
Copy link

@rpetrusha rpetrusha commented Jun 4, 2018

Replaced IComparer example

This PR:

  • Replaces the example, which suffered from a number of limitations. See PR Revised example samples#100.
  • Revises the return value section to remove the table (which renders horribly for IntelliSense).

Fixes dotnet/docs#5587

@rpetrusha rpetrusha self-assigned this Jun 4, 2018
@rpetrusha
Copy link
Author

Closing and reopening to begin new build now that sample code has merged.

@rpetrusha rpetrusha closed this Jun 5, 2018
@rpetrusha rpetrusha reopened this Jun 5, 2018
Copy link
Contributor

@mairaw mairaw left a comment

Choose a reason for hiding this comment

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

LGTM. Just left a question for you @rpetrusha.

<param name="y">The second object to compare.</param>
<summary>Compares two objects and returns a value indicating whether one is less than, equal to, or greater than the other.</summary>
<returns>A signed integer that indicates the relative values of <paramref name="x" /> and <paramref name="y" />, as shown in the following table.
<returns>A signed integer that indicates the relative values of <paramref name="x" /> and <paramref name="y" />. If less than 0, <paramref name="x" /> is less than <paramref name="y" />; if 0, <paramref name="x" /> equals <paramref name="y" />; if greater than 0, <paramref name="x" /> is greater than <paramref name="y" />.
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a bit harder to parse online. did you change this because of IntelliSense?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, @mairaw. I changed it because of IntelliSense. We really need to remove tables from the return value section, since it creates a really broken IntelliSense experience (I keep meaning to open an issue -- I'll do that now). I'm not sure if this is the best format to replace tables with, but I thought that I'd get rid of the table while I was touching the file.

@mairaw
Copy link
Contributor

mairaw commented Jun 6, 2018

Makes sense. I like the changes you made. Hopefully, it looks good for IntelliSense too.

@rpetrusha
Copy link
Author

Thanks, @mairaw. I'm going to merge. Merging master to live should build IntelliSense, so I'll check that on Friday (I'm OOF tomorrow) to see how it looks.

@rpetrusha rpetrusha merged commit 1e521fc into dotnet:master Jun 6, 2018
@rpetrusha rpetrusha deleted the icomparer branch June 6, 2018 22:52
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.

2 participants