Skip to content

Opt into IncludeEvaluationPropertiesAndItems in NullLogger#7386

Merged
Forgind merged 1 commit intodotnet:mainfrom
rainersigwald:fancify-NullLogger
Feb 16, 2022
Merged

Opt into IncludeEvaluationPropertiesAndItems in NullLogger#7386
Forgind merged 1 commit intodotnet:mainfrom
rainersigwald:fancify-NullLogger

Conversation

@rainersigwald
Copy link
Copy Markdown
Member

This keeps a build with all distribution-aware loggers from vetoing the
new behavior because a logger (NullLogger) is attached that doesn't indicate
that it can handle it. It trivially can, so it should!

Related to #1222.

This keeps a build with all distribution-aware loggers from vetoing the
new behavior because a logger (NullLogger) is attached that doesn't indicate
that it can handle it. It trivially can, so it should!
// external contract so can't specify that for the
// argument.

IEventSource4 eventSource4 = (IEventSource4)eventSource;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't IEventSource public? Can't someone have made their own IEventSource that doesn't implement IEventSource4?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would such a thing make it to this point? The engine instantiates the concrete implementation of IEventSource that is passed to loggers at runtime.

I originally wrote this with an as and a null check, but that seems like it would be more likely to silently fail if something very odd happened.

Copy link
Copy Markdown
Contributor

@Forgind Forgind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me!

@Forgind Forgind added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Feb 16, 2022
@Forgind Forgind merged commit def4fbb into dotnet:main Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Logging merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants