Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

Add ISOWeek #963

Merged
merged 1 commit into from
Nov 9, 2018
Merged

Add ISOWeek #963

merged 1 commit into from
Nov 9, 2018

Conversation

terrajobst
Copy link
Member

This fixes #956

@terrajobst terrajobst requested review from a team as code owners November 8, 2018 02:05
@terrajobst
Copy link
Member Author

@tarekgh, any concerns adding this type to .NET Standard?

@terrajobst terrajobst added the netstandard-api This tracks requests for standardizing APIs. label Nov 8, 2018
@terrajobst terrajobst added this to the .NET Standard 2.1 milestone Nov 8, 2018
@jskeet
Copy link

jskeet commented Nov 8, 2018

I'd need to double check the naming guidelines, but shouldn't it be IsoWeek instead of ISOWeek?

@khellang
Copy link
Member

khellang commented Nov 8, 2018

@jskeet This was noted in the API review (and a couple of times since as well) and ended up being caps; https://github.com/dotnet/corefx/issues/28933#issuecomment-392873426

Copy link

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

I'm biased towards using the term WeekYear (and weekYear for parameters etc) to make it really, really clear that it's not the normal calendrical year (e.g. if GetYearStart returns a value with a different year to the year argument) but hopefully it's obvious enough from the class name.

@terrajobst terrajobst merged commit 9474d76 into dotnet:master Nov 9, 2018
@terrajobst terrajobst deleted the iso-week branch November 9, 2018 22:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
netstandard-api This tracks requests for standardizing APIs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include System.Globalization.ISOWeek
9 participants