Log unused targets to internal logger #1084
Conversation
Support object vals for mdlc
Update from base repository
| InternalLogger.Warn("Unused target checking is started... Rule Count: {0}, Target Count: {1}", this.LoggingRules.Count, configuredNamedTargets.Count); | ||
|
|
||
| HashSet<string> targetNamesAtRules = new HashSet<string>(); | ||
| for (int ri = 0; ri < this.LoggingRules.Count; ri++) |
There was a problem hiding this comment.
Looks good, but we can also use easily LINQ here?
Something like (code not checked)
HashSet<string> targetNamesAtRules = new HashSet<string>(this.LoggingRules.SelectMany(r => r.Target).Select(t => t.Name));What do you think?
There was a problem hiding this comment.
It is possible. Maybe I was a bit obsessive about performance, therefore I have used the classic ways.
There was a problem hiding this comment.
Yes it's true that the for is somewhat better in performance. But luckily this code is only run when the config changes.
There was a problem hiding this comment.
You can measure the difference with your stopwatch ;)
There was a problem hiding this comment.
ahaha. Unfortunately I removed my stopwatch :P
OK. Linq way would be more readable.
There was a problem hiding this comment.
To support under version of .Net 3.5, we should not use LINQ. Also Mono.
But it seems we have solution files for 3.5 and newer.
There was a problem hiding this comment.
Hey! Linq is supported, we already use it (and I cannot live without it, I like functional programming)
Are you confused with linq-to-sql?
There was a problem hiding this comment.
No, I am not confused with linq-to-sql.
To be sure whether Linq will broke something, I searched which version has support. Maybe my conclusion is wrong. But I could not found any result for supporting older than .Net 3.5 (without extra effort)
At least Linq MSDN page has not older version.
|
Thanks! Looks really nice! PS: AppVeyor complains about stopwatch. As the current datetime is logged with the internallogger, I think stopwatch isn't needed. |
| InternalLogger.Warn("Unused target checking is canceled -> initialize not started yet."); | ||
| return; | ||
| } | ||
| if (this.InitializeSucceeded.Value == false) |
There was a problem hiding this comment.
We prefer if (!this.InitializeSucceeded.Value). (just code style)
304NotModified
changed the title
Current coverage is
|
Comment about ConfiguredNamedTargets was not clear.
…rnal-logger Log unused targets to internal logger
|
Merged! Thanks! 👍 |
Closes #1074