Skip to content

Conversation

ChrisSchaller
Copy link

The ordering of the enum values is significant, both in terms of real-world calendar systems and the ordinality of the values. The previous document was not sorted by any obvious means, alphabetical might be acceptable at a stretch, but enums in C# have significant values and those values are expected to define their natural order.

Summary

Sorted the Enum members according to their value. Previous implementation was ordered in an ambiguous manner:

  • Friday: 5
  • Monday: 1
  • Saturday: 6
  • Sunday: 0
  • Thursday: 4
  • Tuesday: 2
  • Wedneday: 3

The ordering of the enum values is significant, both in terms of real-world calendar systems and the ordinality of the values. The previous document was not sorted by any obvious means, alphabetical might be acceptable at a stretch, but enums in C# have significant values and those values should are expected to define their _natural_ order.
@ChrisSchaller ChrisSchaller requested a review from a team as a code owner June 2, 2021 02:24
@dnfadmin
Copy link

dnfadmin commented Jun 2, 2021

CLA assistant check
All CLA requirements met.

@ghost ghost added the area-System.Runtime label Jun 2, 2021
@opbld31
Copy link

opbld31 commented Jun 2, 2021

Docs Build status updates of commit b27f765:

✅ Validation status: passed

File Status Preview URL Details
xml/System/DayOfWeek.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:

@BillWagner
Copy link
Member

Thanks for noting this @ChrisSchaller

Unfortunately, I am going to close this PR. As noted in #6749 (comment), our regular CI build that creates these docs from the /// comments will overwrite your change. We're working with the CI team to fix that process.

@gewarren
Copy link
Contributor

This has been fixed!

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.

6 participants