Skip to content

Add missing DateTime/DateTimeOffset/TimeSpan docs #3500

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

Merged
merged 5 commits into from
Nov 15, 2019

Conversation

krwq
Copy link
Member

@krwq krwq commented Nov 15, 2019

Add missing DateTime/DateTimeOffset/TimeSpan docs

cc: @carlossanlop @tarekgh

@carlossanlop carlossanlop added 🏁 Release: .NET Core 2.x Identifies work items for the .NET Core 2.x releases new-content Indicates PRs that contain new articles waiting-on-reviews Indicates PRs that cannot be merged because of the lack of reviews labels Nov 15, 2019
@carlossanlop carlossanlop added this to the November 2019 milestone Nov 15, 2019
Co-Authored-By: Carlos Sanchez Lopez <1175054+carlossanlop@users.noreply.github.com>
krwq and others added 2 commits November 14, 2019 16:55
Co-Authored-By: Carlos Sanchez Lopez <1175054+carlossanlop@users.noreply.github.com>
<returns>To be added.</returns>
<param name="ts">The value to be divided by.</param>
<summary>Returns a new <see cref="T:System.Double" /> value which is the result of division of this instance and the specified <paramref name="ts" />.</summary>
<returns>A new value that represents result of division of this instance by the value of the <paramref name="ts" />.</returns>
Copy link
Member

Choose a reason for hiding this comment

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

value [](start = 23, length = 5)

should be ticks value

Copy link
Member Author

Choose a reason for hiding this comment

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

isn't the result unit-less? We divide time by time so you should get a ratio back

Copy link
Member

@tarekgh tarekgh Nov 15, 2019

Choose a reason for hiding this comment

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

it is not ratio more than real ticks. If you are dividing 2 times what the result means to you? and how you can use it later? it should have some meaning. right?

Copy link
Member Author

Choose a reason for hiding this comment

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

I see, I think this comment was meant to be on the overload above (TimeSpan/double). For this one (TimeSpan/TimeSpan) 4s divided by 2s means it's 2 times longer

Copy link
Member Author

Choose a reason for hiding this comment

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

(note the above one mentions TimeSpan object not value)

<param name="t1">Divident or the value to be divided.</param>
<param name="t2">The value to be divided by.</param>
<summary>Returns a new <see cref="T:System.Double" /> value which is the result of division of <paramref name="t1" /> instance and the specified <paramref name="t2" />.</summary>
<returns>A new value that represents result of division of <paramref name="t1" /> instance by the value of the <paramref name="t2" />.</returns>
Copy link
Member

Choose a reason for hiding this comment

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

value [](start = 23, length = 5)

should be ticks value

@tarekgh
Copy link
Member

tarekgh commented Nov 15, 2019

added some minor comments. otherwise LGTM.

@Thraka
Copy link
Contributor

Thraka commented Nov 15, 2019

@krwq let us know if you're going to make these changes. When you're ready, we'll merge. Thanks!

@krwq
Copy link
Member Author

krwq commented Nov 15, 2019

I'm good with merging as is

@carlossanlop carlossanlop merged commit babd58a into dotnet:master Nov 15, 2019
@mairaw mairaw removed the waiting-on-reviews Indicates PRs that cannot be merged because of the lack of reviews label Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.DateTime 🏁 Release: .NET Core 2.x Identifies work items for the .NET Core 2.x releases new-content Indicates PRs that contain new articles
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants