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

Use type's namespace instead of assembly FullName #777

Closed
wants to merge 1 commit into from

Conversation

khellang
Copy link
Contributor

@khellang khellang commented Apr 27, 2016

Same as #776, but targeting release. Fixes #775

@mikeharder
Copy link
Contributor

@davidfowl: I can merge this to release and dev if you give it a :shipit: and get it approved for RC2.

@mikeharder mikeharder self-assigned this Apr 27, 2016
@halter73
Copy link
Member

:shipit:

@mikeharder
Copy link
Contributor

@davidfowl: The previous value was the namespace Microsoft.AspNetCore.Server.Kestrel rather than the type Microsoft.AspNetCore.Server.Kestrel.KestrelServer. Which do we want going forward?

@davidfowl
Copy link
Member

The former one looks nicer

@khellang
Copy link
Contributor Author

Actually, why not just use typeof(KestrelServer).Name? What is CreateLogger <T> using?

@halter73
Copy link
Member

@khellang I agree that Microsoft.AspNetCore.Server.Kestrel looks nicer. It also makes the most sense to me, because the KestrelServer class isn't doing any logging directly.

@khellang khellang force-pushed the type-name-release branch from 3f1010b to 9eb3fb8 Compare April 29, 2016 07:53
@khellang
Copy link
Contributor Author

Aight. Changed to namespace. Or did you just want med to hard code Microsoft.AspNetCore.Server.Kestrel?

Anything looks better than Microsoft.AspNetCore.Server.Kestrel, Version=1.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60 😉

@davidfowl
Copy link
Member

Yah, hardcode it

@khellang khellang force-pushed the type-name-release branch from 9eb3fb8 to 35ddb36 Compare April 29, 2016 08:11
@khellang
Copy link
Contributor Author

Hardcoded it is 😁

@khellang khellang force-pushed the type-name-release branch from 35ddb36 to f769d01 Compare April 29, 2016 13:07
@davidfowl
Copy link
Member

@Eilon suggested we do typeof(KestrelServer).Namespace

@khellang
Copy link
Contributor Author

khellang commented Apr 29, 2016

Fffffffuuuuu. That's what I had before hard coding it 😝

Updated.

@khellang khellang force-pushed the type-name-release branch from f769d01 to eb595b5 Compare April 29, 2016 16:17
@khellang khellang changed the title Use type FullName instead of assembly FullName Use type's namespace instead of assembly FullName Apr 29, 2016
@Eilon
Copy link
Member

Eilon commented Apr 29, 2016

Yeah don't listen to @davidfowl 😄

@benaadams
Copy link
Contributor

Is typeof(KestrelServer).Namespace a compile time constant? (and if not why not...)

@Eilon
Copy link
Member

Eilon commented Apr 29, 2016

@benaadams it's not, but I think in this case it's once per app.

@benaadams
Copy link
Contributor

@Eilon raised issue dotnet/roslyn#10972

@mikeharder
Copy link
Contributor

We decided to take this change for post-RC2. Manually merged to dev in 8a98402.

@mikeharder mikeharder closed this Apr 29, 2016
@khellang khellang deleted the type-name-release branch May 26, 2016 22:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants