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

Implement IDownstreamApi overloads that take JsonTypeInfo<T> as a parameter to enable source generated Json deserialization for NativeAOT #2959

Merged
merged 10 commits into from
Aug 27, 2024
Merged

Implement IDownstreamApi overloads that take JsonTypeInfo<T> as a parameter to enable source generated Json deserialization for NativeAOT #2959

merged 10 commits into from
Aug 27, 2024

Conversation

dualtagh
Copy link
Contributor

@dualtagh dualtagh commented Jul 31, 2024 •
edited
Loading

Addresses #2930

  • Update Microsoft.Identity.Abstractions to 7.0.0
  • Implementation for new DownstreamApi interfaces in Microsoft.Identity.Abstractions for source gen Json serialization. Implementaitons are generated or copied as the existing with just the addition of jsonTypeInfo params

@dualtagh dualtagh force-pushed the dualtagh/downstreamapi_aot branch 2 times, most recently from d3eb178 to cb1510a Compare August 21, 2024 15:17
@dualtagh dualtagh marked this pull request as ready for review August 21, 2024 15:39
@dualtagh dualtagh requested a review from a team as a code owner August 21, 2024 15:39
@dualtagh dualtagh mentioned this pull request Aug 22, 2024
Open
jmprieur
jmprieur approved these changes Aug 22, 2024
View reviewed changes
Copy link
Collaborator

@jmprieur jmprieur 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.

Thanks @dualtagh. Nice job!

I approved but requested that you don't special case the reference for net8.0 and net9.0 in the Directory.Build.props, but just use Abstractions 7.0.0 for all target framework (change line 91).

Otherwise this will break our unified / id4sbuild
cc: @jennyf19

Directory.Build.props Outdated Show resolved Hide resolved
tests/Microsoft.Identity.Web.Test/Microsoft.Identity.Web.Test.csproj Outdated Show resolved Hide resolved
jennyf19
jennyf19 approved these changes Aug 26, 2024
View reviewed changes
Copy link
Collaborator

@jennyf19 jennyf19 left a comment

Choose a reason for hiding this comment

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

Great to see the additional test coverage @dualtagh
:shipit:

@dualtagh dualtagh merged commit c8ba0ce into master Aug 27, 2024
5 checks passed
@dualtagh dualtagh deleted the dualtagh/downstreamapi_aot branch August 27, 2024 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmprieur jmprieur jmprieur approved these changes

@jennyf19 jennyf19 jennyf19 approved these changes

@pmaytak pmaytak pmaytak approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@dualtagh @jmprieur @jennyf19 @pmaytak