Add event names for logs (#6818) #4495
Conversation
There was a problem hiding this comment.
How visible will these new names be? If they'll appear in the output, we might want to be more consistent about Starting… -> Done… versus Before… -> After… or other conventions we've used. But, I don't see a need to block this PR for decisions on the right convention.
| "Could not bind to collection using a format like {ModelName}=value1&{ModelName}=value2"); | ||
|
|
||
|
|
| "Executed action method {ActionName}, returned result {ActionResult} in {ElapsedMilliseconds}ms."); | ||
|
|
||
| _logFilterExecutionPlan = LoggerMessage.Define<string, string[]>( | ||
| LogLevel.Debug, | ||
| 1, | ||
| new EventId(1, "LogFilterExecutionPlan"), |
There was a problem hiding this comment.
This name doesn't look like any of the others. Suggest FilterExecutionPlan or OrderedFilterExecutionPlan.
| 5, | ||
| "Located compiled view for view at path '{Path}'."); | ||
|
|
||
| _viewCompilerLocatedCompiledViewForPath = LoggerMessage.Define<string>( |
From aspnet/Mvc#6818 (comment), the names appear in the log output and used for filtering. I guess it's important that it's unique, but the issue doesn't say much about the nomenclature. |
pranavkm
force-pushed
the
prkrishn/6818
branch
from
fde620c
to
63355cb
Compare
|
@Kahbazi thanks for the PR |
|
FYI, we try to avoid using |
|
@pranavkm you're welcome. Should I remove the |
Go for it! |
Fixes aspnet/Mvc#6818