Skip to content
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

Expose the constants in TimeSpan #103993

Merged
merged 1 commit into from
Jun 26, 2024
Merged

Conversation

IDisposable
Copy link
Contributor

Publicly expose the useful constants that TimeSpan had internally declared since they're rooted in hard reality and not changeable, and quite useful elsewhere. Everything is a long except HoursPerDay because that is useful in many non-TimeSpan contexts and rarely would overflow.

Added /// documentation for newly public constants and updated System.Runtime.cs assembly API reference.

Use the newly exposed values in all the runtime places similar constants are declared in runtime EXCEPT did not use the TimeSpan constant in EventLogInternal.cs since that source is also built for older frameworks, leave existing internal constant.

In Calendar.cs, use an int cast at the call-sites to Add method to prevent performance degradation, these calls are known not to overflow.

Fixes #94545

Publicly expose the useful constants that `TimeSpan` had internally declared since they're rooted in hard reality and not changeable, and quite useful elsewhere. Everything is a `long` except `HoursPerDay` because that is useful in many non-TimeSpan contexts and rarely would overflow.

Added /// documentation for newly public constants and updated _System.Runtime.cs_ assembly API reference.

Use the newly exposed values in all the runtime places similar constants are declared in runtime **EXCEPT** did not use the `TimeSpan` constant in _EventLogInternal.cs_ since that source is also built for older frameworks, leave existing internal constant.

In _Calendar.cs_, use an `int` cast at the call-sites to `Add` method to prevent performance degradation, these calls are known not to overflow.

Fixes dotnet#94545
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

1 similar comment
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

Copy link
Member

@tarekgh tarekgh left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @IDisposable!

@tarekgh
Copy link
Member

tarekgh commented Jun 26, 2024

/ba-g the failures are already tracked in the dnceng dotnet/dnceng#3007.

@tarekgh tarekgh merged commit 1d272e6 into dotnet:main Jun 26, 2024
127 of 146 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 27, 2024
@IDisposable IDisposable deleted the timespan-constants branch August 23, 2024 23:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.DateTime community-contribution Indicates that the PR has been added by a community member new-api-needs-documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[API Proposal]: Expose the remaining const values in TimeSpan
2 participants