SendGrid ServiceProvider Overload #2301
Conversation
|
idk who approved the workflow previously, but thank you 😄 I have updated the public api txt file |
|
can i get this merged? |
There was a problem hiding this comment.
@cmeyertons thank you for your contribution! Overall it LGTM, but please answer to my test question. Thanks!
| var check = registration.Factory(serviceProvider); | ||
|
|
||
| registration.Name.ShouldBe("sendgrid"); | ||
| check.ShouldBeOfType<SendGridHealthCheck>(); |
There was a problem hiding this comment.
Do I read this correctly that this test is not veryfing if value my_api_key has been actually used by the health check?
There was a problem hiding this comment.
i used the same assertion code that was acceptable for the previous test.
| @@ -31,13 +31,41 @@ public static IHealthChecksBuilder AddSendGrid( | |||
| IEnumerable<string>? tags = default, | |||
| TimeSpan? timeout = default) | |||
| { | |||
| var registrationName = name ?? NAME; | |||
| return builder.AddSendGrid(_ => apiKey, name, failureStatus, tags, timeout); | |||
There was a problem hiding this comment.
nit: we are now capturing a closure (so there is a performance) penalty, but it helps to reduce code duplication, so I am fine with that. We may however, change that approach in the future.
…-provider-overload
test/HealthChecks.SendGrid.Tests/DependencyInjection/RegistrationTests.cs
Outdated
Show resolved
Hide resolved
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #2301 +/- ##
==========================================
- Coverage 64.25% 64.02% -0.24%
==========================================
Files 248 247 -1
Lines 8399 8291 -108
Branches 590 574 -16
==========================================
- Hits 5397 5308 -89
+ Misses 2853 2839 -14
+ Partials 149 144 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
What this PR does / why we need it:
Adds an
IServiceProvideroverload for configuring SendGrid to allow for ApiKey to be pulled from DI (via options pattern, etc)Which issue(s) this PR fixes:
Please reference the issue this PR will close: #2300
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Not really, no breakages, just a new overload
Please make sure you've completed the relevant tasks for this PR, out of the following list: