-
Notifications
You must be signed in to change notification settings - Fork 340
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.
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
Remove net6win #4567
Remove net6win #4567
Conversation
626235d
to
912bd36
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Anymore changes that need to be done?
Any way we can update exceptions in the code with more details related to this change? If net6.0-windows PCA app gets updated to the new MSAL version, standard exceptions will be thrown to use Broker or Desktop packages; but if the user didn't read the release notes, it'll be unclear why the change happened.
src/client/Microsoft.Identity.Client/Microsoft.Identity.Client.csproj
Outdated
Show resolved
Hide resolved
tests/Microsoft.Identity.Test.Integration.Win8/BrokerOnWin8Tests.cs
Outdated
Show resolved
Hide resolved
tests/Microsoft.Identity.Test.Unit/Kerberos/KerberosSupplementalTicketManagerTests.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Microsoft.Identity.Client.csproj
Outdated
Show resolved
Hide resolved
tests/Microsoft.Identity.Test.Integration.Win8/BrokerOnWin8Tests.cs
Outdated
Show resolved
Hide resolved
tests/Microsoft.Identity.Test.Unit/AppConfigTests/PublicClientApplicationBuilderTests.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple of minor questions. LGTM otherwise
so for people are targeting |
842a7f4
to
7cae0eb
Compare
@bgavrilMS so it this a breaking change? #4567 (comment) |
7cae0eb
to
83ebad4
Compare
@bgavrilMS Are these 3 PRs ready for review (this, uwp, xamarin removal)? |
Yes, we discussed with @localden about removing UWP. Xamarin goes out of support May 1st. And we discussed about net6-win too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions and questions.
src/client/Microsoft.Identity.Client/AppConfig/PublicClientApplicationBuilder.cs
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ApiConfig/AcquireTokenInteractiveParameterBuilder.cs
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ApiConfig/AcquireTokenInteractiveParameterBuilder.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/AppConfig/PublicClientApplicationBuilder.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created an docs issue to update the docs: MicrosoftDocs/microsoft-authentication-library-dotnet#395 |
…eractiveParameterBuilder.cs Co-authored-by: Peter <[email protected]>
…licationBuilder.cs Co-authored-by: Peter <[email protected]>
* Attempt * fix * fix * Remove NET_ONLY * Adress PR comments * Address PRs * Update src/client/Microsoft.Identity.Client/ApiConfig/AcquireTokenInteractiveParameterBuilder.cs Co-authored-by: Peter <[email protected]> * Update src/client/Microsoft.Identity.Client/AppConfig/PublicClientApplicationBuilder.cs Co-authored-by: Peter <[email protected]> --------- Co-authored-by: Peter <[email protected]>
Fixes #4468
And this PR updates the sample (also bumps it to net7, because net6 is out of support)
Azure-Samples/ms-identity-dotnetcore-maui#16