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

LINQ: Fixes preserve DateTime.Kind when passing value to custom JsonConverter #3224

Merged
merged 13 commits into from
Jun 6, 2022
Merged

LINQ: Fixes preserve DateTime.Kind when passing value to custom JsonConverter #3224

merged 13 commits into from
Jun 6, 2022

Conversation

ccurrens
Copy link
Contributor

Description

This commit updates ExpressionToSql.ApplyCustomConverter to preserve the DateTimeKind when parsing a DateTime that is passed to a custom JsonConverter.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Closing issues

closes #3222

@ccurrens
Copy link
Contributor Author

Happy to add unit tests, but not sure where they should be put

@ccurrens ccurrens changed the title LINQ: preserve DateTime.Kind when passing to custom JsonConverter LINQ: preserve DateTime.Kind when passing value to custom JsonConverter May 26, 2022
@j82w
Copy link
Contributor

j82w commented May 26, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@ccurrens ccurrens requested a review from ealsur May 26, 2022 17:39
@ccurrens ccurrens requested a review from ealsur May 27, 2022 12:06
@j82w
Copy link
Contributor

j82w commented May 27, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

ealsur
ealsur previously approved these changes May 27, 2022
View reviewed changes
Copy link
Member

@ealsur ealsur left a comment

Choose a reason for hiding this comment

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

@khdang - Could you please review from the LINQ side?

@ealsur
Copy link
Member

ealsur commented May 31, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

leminh98
leminh98 previously approved these changes Jun 2, 2022
View reviewed changes
Copy link
Contributor

@leminh98 leminh98 left a comment

Choose a reason for hiding this comment

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

:shipit:

@ccurrens ccurrens dismissed stale reviews from leminh98 and ealsur via d87396b June 3, 2022 13:42
@j82w
Copy link
Contributor

j82w commented Jun 3, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@ealsur ealsur changed the title LINQ: preserve DateTime.Kind when passing value to custom JsonConverter LINQ: Fixes preserve DateTime.Kind when passing value to custom JsonConverter Jun 3, 2022
@ealsur
Copy link
Member

ealsur commented Jun 3, 2022 •
edited
Loading

@ccurrens based on the test failure, can you run \UpdateContracts.ps1 locally? That will generate the updated baseline files? Or could it be because of the timezone difference between your local machine and the timezone of the machine that runs the gates?

image

@ccurrens
Copy link
Contributor Author

ccurrens commented Jun 3, 2022

I did before. This looks like an issue because the timezone is different between my machine and the runner. Will probably need to exclude the timezone.

@ealsur
Copy link
Member

ealsur commented Jun 3, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@ealsur
Copy link
Member

ealsur commented Jun 3, 2022

If all the tests pass, we'll merge this

@j82w
Copy link
Contributor

j82w commented Jun 3, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@ealsur ealsur enabled auto-merge (squash) June 3, 2022 20:46
leminh98
leminh98 approved these changes Jun 6, 2022
View reviewed changes
Copy link
Contributor

@leminh98 leminh98 left a comment

Choose a reason for hiding this comment

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

:shipit:

sboshra
sboshra approved these changes Jun 6, 2022
View reviewed changes
Copy link
Contributor

@sboshra sboshra left a comment

Choose a reason for hiding this comment

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

:shipit:

@ealsur ealsur merged commit dfa0b7c into Azure:master Jun 6, 2022
@ccurrens
Copy link
Contributor Author

ccurrens commented Jun 6, 2022

Thanks for the quick and constant communication on this, and thanks for the merge!

@ccurrens ccurrens deleted the dates-in-linq branch June 6, 2022 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leminh98 leminh98 leminh98 approved these changes

@sboshra sboshra sboshra approved these changes

@ealsur ealsur ealsur approved these changes

@khdang khdang Awaiting requested review from khdang khdang is a code owner

@j82w j82w Awaiting requested review from j82w

@kirankumarkolli kirankumarkolli Awaiting requested review from kirankumarkolli kirankumarkolli is a code owner

@neildsh neildsh Awaiting requested review from neildsh neildsh is a code owner

@FabianMeiswinkel FabianMeiswinkel Awaiting requested review from FabianMeiswinkel FabianMeiswinkel is a code owner

@kirillg kirillg Awaiting requested review from kirillg kirillg is a code owner

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

Successfully merging this pull request may close these issues.

SDK modifies values passed to custom JsonConverters in LINQ queries
5 participants
@ccurrens @j82w @ealsur @sboshra @leminh98