Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

NullLoggerFactory and friends are in the wrong namespace #594

Closed
rynowak opened this issue Apr 11, 2017 · 11 comments
Closed

NullLoggerFactory and friends are in the wrong namespace #594

rynowak opened this issue Apr 11, 2017 · 11 comments
Assignees
Labels
Investigate Investigation item wontfix

Comments

@rynowak
Copy link
Member

rynowak commented Apr 11, 2017

All of these types are in the Microsoft.Extensions.Logging.Abstractions - whereas the types that they implement are in Microsoft.Extensions.Logging.

See: #566 - I don't see any discussion of the namespace in this issue or the PR which leads me to believe it's not intentional.

@muratg
Copy link

muratg commented Apr 11, 2017

cc @ajaybhargavb @Eilon @davidfowl

@Eilon
Copy link
Member

Eilon commented Apr 11, 2017

I think we did see this while @ajaybhargavb was doing all the NullLoggerXyz cleanup, but perhaps it was too big of a breaking change? I don't recall exactly...

@ajaybhargavb
Copy link
Contributor

I don't remember what exactly but some of these types were used in repos that only needed to depend on Microsoft.Extensions.Logging.Abstractions.

@davidfowl
Copy link
Member

We should fix it.

@rynowak
Copy link
Member Author

rynowak commented Apr 11, 2017

I don't remember what exactly but some of these types were used in repos that only needed to depend on Microsoft.Extensions.Logging.Abstractions.

The issue I'm raising here is that the namespace is wrong.

@Eilon
Copy link
Member

Eilon commented Apr 11, 2017

@ajaybhargavb can you compare what was shipped in 1.1.x to what we have in 2.0-dev?

@muratg muratg added this to the 2.0.0-preview1 milestone Apr 11, 2017
@muratg muratg added 1 - Ready Investigate Investigation item labels Apr 11, 2017
@muratg
Copy link

muratg commented Apr 11, 2017

Assigned to @ajaybhargavb for investigation.

@ajaybhargavb
Copy link
Contributor

image

From the comparison, the namespace NullLoggerFactory is in is consistent with NullLogger and NullLoggerProvider from rel/1.1.2.

Do we want to change all of those to Microsoft.Extensions.Logging?

@muratg
Copy link

muratg commented Apr 21, 2017

@ajaybhargavb will this fit in preview1? If so please bring this back to 2.0.0-preview1 milestone. Thanks.

@ajaybhargavb
Copy link
Contributor

I'll see how many repos will need the update. But first, are we okay with this breaking change?

@Eilon
Copy link
Member

Eilon commented Apr 21, 2017

Talked with @muratg and @ajaybhargavb and given that this would be an annoying breaking for almost no value, so closing it.

@Eilon Eilon closed this as completed Apr 21, 2017
@Eilon Eilon added wontfix and removed 2 - Working labels Apr 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Investigate Investigation item wontfix
Projects
None yet
Development

No branches or pull requests

5 participants