ServiceProviderEngineScope._disposables is intended to be lazy

aspnet / DependencyInjection

[Archived] Contains common DI abstractions that ASP.NET Core and Entity Framework Core use. Project moved to https://github.com/aspnet/Extensions

Apache License 2.0

877 stars 320 forks source link

Closed dfederm closed 6 years ago

dfederm commented 6 years ago

_disposables is set in the implicit ctor and is never set to null, so this change just removes the nullchecks on it.

Admittedly the perf benefit is nominal, but it's a safe change, so why not.

Eilon commented 6 years ago

Looks like CLA bot is happy now?

dfederm commented 6 years ago

Yup, rebasing kicked it and reevaluated the status it seems.

dfederm commented 6 years ago

Travis build seems broken though :-(

Eilon commented 6 years ago

I think that Travis issue is a known issue and not related to the PR.

We'll have a team member take a look at the PR.

@muratg - can you assign someone to take a look?

pakrym commented 6 years ago

Setting _disposables in ctor is a bug, it should be lazily initialized.

dfederm commented 6 years ago

@pakrym Adjusted the PR for the new understanding of the intent.

pakrym commented 6 years ago

LGTM, I'll merge when passes.

dfederm commented 6 years ago

@pakrym I'll try kicking it again, but the CI build seems broken, even on the dev branch.

Eilon commented 6 years ago

It's just Travis Mac that's failing. Travis Linux and AppVeyor are both passing. The Travis Mac failure is almost certainly unrelated to this PR.

pakrym commented 6 years ago

@dfederm Thank you.