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.

Sign up for GitHub

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

Jump to bottom

Add rawLocalTimestamp to new interface #1924

Merged
merged 8 commits into from
Apr 1, 2020
Merged

Add rawLocalTimestamp to new interface #1924

merged 8 commits into from
Apr 1, 2020

Conversation

EricDahlvang
Copy link
Member

Fixes #1612

@coveralls
Copy link

coveralls commented Mar 18, 2020 •
edited
Loading

Pull Request Test Coverage Report for Build 116987

  • 19 of 23 (82.61%) changed or added relevant lines in 3 files are covered.
  • 10 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.02%) to 81.483%
Changes Missing Coverage Covered Lines Changed/Added Lines %
libraries/botbuilder/src/channelServiceRoutes.ts 1 5 20.0%
Files with Coverage Reduction New Missed Lines %
libraries/botbuilder/src/channelServiceRoutes.ts 1 1.9%
libraries/botframework-streaming/src/webSocket/webSocketTransport.ts 2 80.65%
libraries/botframework-streaming/src/webSocket/webSocketServer.ts 7 66.67%
Totals Coverage Status
Change from base Build 116832: -0.02%
Covered Lines: 9338
Relevant Lines: 11040
💛 - Coveralls

stevengum
stevengum reviewed Mar 18, 2020
View reviewed changes
Copy link
Member

@stevengum stevengum left a comment

Choose a reason for hiding this comment

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

Not a terrific fan of adding something to the Activity schema...

libraries/botbuilder/src/activityValidator.ts Outdated Show resolved Hide resolved
libraries/botbuilder/src/activityValidator.ts Outdated Show resolved Hide resolved
libraries/botbuilder/src/activityValidator.ts Outdated Show resolved Hide resolved
@EricDahlvang
Copy link
Member Author

Not a terrific fan of adding something to the Activity schema...

I'm not a fan of this either. It's a 'one-off' for javascript, because of the UTC Date object behavior. If you have some other ideas, let's discuss.

stevengum
stevengum approved these changes Apr 1, 2020
View reviewed changes
Copy link
Member

@stevengum stevengum left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@EricDahlvang this looks great! After a happy-path test for Activity.timestamp is added this PR looks ready to go in.

@Stevenic, @johnataylor do you have any comments on naming?

@EricDahlvang EricDahlvang requested a review from johnataylor April 1, 2020 20:13
@EricDahlvang EricDahlvang merged commit 3017719 into master Apr 1, 2020
@EricDahlvang EricDahlvang deleted the eric/addlocalTimezoneOffset branch April 1, 2020 21:43
@EricDahlvang EricDahlvang changed the title Add localTimezoneOffset to Activity interface Add localTimezoneOffset to new interface Apr 1, 2020
@EricDahlvang EricDahlvang changed the title Add localTimezoneOffset to new interface Add rawTimezoneOffset to new interface Apr 1, 2020
@EricDahlvang EricDahlvang changed the title Add rawTimezoneOffset to new interface Add rawLocalTimestamp to new interface Apr 1, 2020
@mdrichardson mdrichardson mentioned this pull request May 12, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevengum stevengum stevengum approved these changes

@johnataylor johnataylor johnataylor approved these changes

@christopheranderson christopheranderson Awaiting requested review from christopheranderson

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@EricDahlvang @coveralls @johnataylor @stevengum