Skip to content
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

Make the level mapping routines public #141

Merged
merged 1 commit into from
May 21, 2019

Conversation

nblumhardt
Copy link
Member

If there's any library where this belongs, I guess it's here. Resolves #81.

No tests, as they'd be pretty much an exact copy of the already-declarative code.

@nblumhardt
Copy link
Member Author

Any thoughts on this, @serilog/reviewers-core ?

@tsimbalar
Copy link
Member

LGTM ! 👍

I see it as extremely tempting to make them extension methods , but I suppose that YAGNI tells me that NO 🤷‍♂

(and if we were to do it, should the namespace be any different ? 🤔 )

@nblumhardt
Copy link
Member Author

I thought about this, but came out on the side of restraint since usage will be very, very infrequent (as far as I can tell). Extension methods do bring all kinds of namespace pollution concerns into the picture :-) .. Might just side-step that and :shipit:

@nblumhardt nblumhardt merged commit 29bdf6c into serilog:dev May 21, 2019
@nblumhardt
Copy link
Member Author

Thanks for the review, BTW! :-)

@xrkolovos
Copy link

when will this feature be released?

@nblumhardt
Copy link
Member Author

@xrkolovos thanks for the nudge; no plan yet, but #145 is the PR to watch.

@nblumhardt nblumhardt mentioned this pull request Aug 19, 2019
@nblumhardt nblumhardt added this to the 3.0.0 milestone Aug 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make function ConvertLevel public availabe so we can use it in our code
3 participants