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

[PORT] Port enhancements to take/skip/resolve/compare functions #2991

Merged
merged 7 commits into from
Nov 3, 2020
Merged

[PORT] Port enhancements to take/skip/resolve/compare functions #2991

merged 7 commits into from
Nov 3, 2020

Conversation

Danieladu
Copy link
Contributor

@Danieladu Danieladu commented Nov 3, 2020 •
edited
Loading

Description

Port some built-in function related updates into js SDK

take/skip

Tolerate over-range operator in skip/take function
Original PR: microsoft/botbuilder-dotnet#4879

resolve

Add resolve function for resolving TimexProperty values to string values
Original PR: microsoft/botbuilder-dotnet#4573

greaterThan, greaterThanOrEqual, lessThan, lessThanOrEqual

List of affected functions：greaterThan, greaterThanOrEqual, lessThan, lessThanOrEqual
Add support of logical comparison in Adaptive-Expressions can compare Date object.
Original PR: microsoft/botbuilder-dotnet#4788

status

  • take/skip
  • resolve
  • greaterThan, greaterThanOrEqual, lessThan, lessThanOrEqual

@Danieladu Danieladu changed the title [Port] Port some Expression changes in C# into js [Port] Port updates to take/skip/json functions Nov 3, 2020
@Danieladu Danieladu changed the title [Port] Port updates to take/skip/json functions [Port] Port updates to take/skip/json/resolve functions Nov 3, 2020
@cosmicshuai
Copy link
Contributor

Verified JSON error msg PR is no need to port. JSON.parse function will throw correct info about the position once an error encountered.

@boydc2014
Copy link
Contributor

Verified JSON error msg PR is no need to port. JSON.parse function will throw correct info about the position once an error encountered.

OK, cool, removing the json part now

@boydc2014 boydc2014 changed the title [Port] Port updates to take/skip/json/resolve functions [Port] Port updates to take/skip/resolve/compare functions Nov 3, 2020
@Danieladu Danieladu marked this pull request as ready for review November 3, 2020 09:10
@boydc2014 boydc2014 changed the title [Port] Port updates to take/skip/resolve/compare functions [Port] Port enhancements to take/skip/resolve/compare functions Nov 3, 2020
@cosmicshuai
keep formatDateTime function returns the same results with C# and add…
fa598e4 
… a lg test for resolve timex (#2994)

* add timex resolve function

* fix formatDateTime function behavior
@boydc2014
Copy link
Contributor

boydc2014 commented Nov 3, 2020 •
edited
Loading

Hey, @joshgummersall, can you help check this one. This is a purely porting PR to align some functions behavior in expression behaviors with C#, without any changes in core logic.

Since it touches the package.json (updating a dependency Recognizers-Text to 1.3.0), it will need your review and approval. And after this gets in, we hope to do a cherry pick of this into R11.

Sorry the late schedule, the reason for this is, we just learn about the previous auto-porting bot stop working, so we did a full check of R11 PRs to make sure we didn't miss any PR to port. And we found a few PRs listed above to be ported. Thanks.

@boydc2014 boydc2014 changed the title [Port] Port enhancements to take/skip/resolve/compare functions [PORT] Port enhancements to take/skip/resolve/compare functions Nov 3, 2020
joshgummersall
joshgummersall approved these changes Nov 3, 2020
View reviewed changes
Copy link
Contributor

@joshgummersall joshgummersall left a comment

Choose a reason for hiding this comment

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

Looks good, I'll set up a cherrypick PR once the main build passes.

@joshgummersall joshgummersall merged commit 708bc57 into main Nov 3, 2020
@joshgummersall joshgummersall deleted the port branch November 3, 2020 17:16
joshgummersall pushed a commit that referenced this pull request Nov 3, 2020
@Danieladu @feich-ms
@cosmicshuai @boydc2014
[PORT] Port enhancements to take/skip/resolve/compare functions (#2991)
a8b247a 
* Tolerate over-range operator in skip/take function

* Fully respect the original input in a string interpolation context

* support date type comparison in expression

* add timex resolve function (#2993)

* revert the escape change

* keep formatDateTime function returns the same results with C# and add a lg test for resolve timex (#2994)

* add timex resolve function

* fix formatDateTime function behavior

Co-authored-by: Fei Chen <[email protected]>
Co-authored-by: Shuai Wang <[email protected]>
Co-authored-by: Dong Lei <[email protected]>
@joshgummersall
Copy link
Contributor

@boydc2014 it looks like this package group is referenced in a few other places as well: https://github.com/microsoft/botbuilder-js/search?l=JSON&q=recognizers

Do we need to update these as well?

@stevkan stevkan mentioned this pull request Jul 21, 2021
Closed
@ryub3n
Copy link

ryub3n commented Aug 9, 2021

Hi, I am having issues with the typing indicator and it seems like the message sorting issue could be the culprit as mentioned by @stevkan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshgummersall joshgummersall joshgummersall approved these changes

@boydc2014 boydc2014 boydc2014 approved these changes

@stevengum stevengum Awaiting requested review from stevengum

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Danieladu @cosmicshuai @boydc2014 @joshgummersall @ryub3n @feich-ms