Skip to content

Conversation

@Crown0815
Copy link
Contributor

@Crown0815 Crown0815 commented Apr 25, 2019

Summary

It took some time for me to understand why the last example has -08:00 as offset. Since the example seemed to be about the addition of a TimeSpan the change in offset was quite confusing.

Therefore, I split the example into two parts, one only focusing on the addition of a TimeSpan and the second on the adaption of the daylight time specific time zone.

I also fixed a typo in the year given in line 84.

Fixes dotnet/dotnet-api-docs#2355

It took some time for me to understand why the last example has -08:00 as offset. Since the example seemed to be about the addition of a TimeSpan the change in offset was quite confusing. 

Therefore, I split the example into two parts, one only focusing on the addition of a TimeSpan and the second on the adaption of the daylight time specific time zone. 

I also fixed a typo in the year given in line 84.
@Crown0815 Crown0815 requested a review from BillWagner as a code owner April 25, 2019 06:15
@BillWagner BillWagner added the ✨ 1st-time samples contributor! Indicates PRs from new contributors to the samples repository label Apr 26, 2019
@BillWagner
Copy link
Member

Thanks for making this update @Crown0815 The C# changes look great.

Can you make the corresponding update to the VisualBasic sample? The source is here: https://github.com/dotnet/samples/blob/master/snippets/visualbasic/VS_Snippets_CLR_System/system.DateTimeOffset.Operators/vb/Operators.vb#L113

After that, we'll merge this change.

/cc @rpetrusha

Adaption of changes made to the C# example in commit e1833f1 to the Visual Basic sample base.
This change was made based on the feedback received to [PR dotnet#850](dotnet#850 (comment))
@Crown0815 Crown0815 requested a review from rpetrusha as a code owner April 28, 2019 19:19
@dnfclas
Copy link

dnfclas commented Apr 28, 2019

CLA assistant check
All CLA requirements met.

@Crown0815
Copy link
Contributor Author

Thank you very much for your feedback @BillWagner.

I adopted my changes to the C# sample to the VisualBasic sample as well. Please let me know if any further adoptions are necessary.

I am very grateful for the opportunity to contribute.

Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

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

Thanks @Crown0815

I've reviewed both sets of changes, and this is ready to :shipit:

I've changed the base branch to "staging", and I'll merge this now. It will go live next week, during Build.

@BillWagner BillWagner changed the base branch from master to staging April 30, 2019 13:14
@BillWagner BillWagner merged commit c026fa2 into dotnet:staging Apr 30, 2019
Thraka pushed a commit that referenced this pull request May 1, 2019
* Clarify DateTime implicit conversion example

It took some time for me to understand why the last example has -08:00 as offset. Since the example seemed to be about the addition of a TimeSpan the change in offset was quite confusing. 

Therefore, I split the example into two parts, one only focusing on the addition of a TimeSpan and the second on the adaption of the daylight time specific time zone. 

I also fixed a typo in the year given in line 84.

* Clarify DateTime implicit conversion VB example

Adaption of changes made to the C# example in commit e1833f1 to the Visual Basic sample base.
This change was made based on the feedback received to [PR #850](#850 (comment))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ 1st-time samples contributor! Indicates PRs from new contributors to the samples repository

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mistake in sample for DateTimeOffset.Implicit

3 participants