-
Notifications
You must be signed in to change notification settings - Fork 218
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
add net461 target for cache extensions #737
Conversation
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.
This is awesome @jennyf19, and this opens the way to delivering even more value for the ASP.NET customers.
@henrik-me : is it an option to support .NET 4.7.2 instead of .NET 4.6.1 ? this would also give us the certificate management with only one line of conditional compilation (with 4.6.1 the crypto is a bit different)
4.7.2 is recommended in many ways. Not sure every one is there yet though and what the timeline will be (probably most by end of CY). Will require a bit of investigation on current usages. Perhaps we can start with 4.7.2? |
@henrik-me @jmprieur ok. thanks. |
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.
perhaps a utest validating that this the expected is available for the 4.7.2 target. #Resolved |
We could even expose the certificate management (including getting them from keyVault), by changing one line in DefaultCertificateLoader.cs: line 97 |
i'm going to add a test app instead. so we can verify all the classes taht are added e2e. In reply to: 720773691 [](ancestors = 720773691) |
maybe in a next PR? In reply to: 721111217 [](ancestors = 721111217) |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
I don't really like the test apps for something like this, but can be clarified as required validation step when checking the API. In reply to: 721338397 [](ancestors = 721338397,720773691) |
yes, this is demonstrated in the sample: Azure-Samples/ms-identity-aspnet-webapp-openidconnect#43 |
@henrik-me #754 for adding test. |
@jmprieur as discussed