feat: adding cache to dotnet --info content #263
Conversation
phmonte
force-pushed
the
feature/dotnetinfo-cache
branch
from
215d9cb to
2cf3728
Compare
| private static Dictionary<string, List<string>> cache = new Dictionary<string, List<string>>(); | ||
|
|
||
| [Pure] | ||
| public static List<string> GetCache(string path) |
There was a problem hiding this comment.
IReadOnlyList or IReadOnlyCollection
| [Pure] | ||
| public static List<string> GetCache(string path) | ||
| { | ||
| return cache.FirstOrDefault(x => x.Key == path).Value; |
There was a problem hiding this comment.
On null this will fail. And we have a dictionary, so please use cache.TryGet()
| return cache.FirstOrDefault(x => x.Key == path).Value; | ||
| } | ||
|
|
||
| [Pure] |
There was a problem hiding this comment.
@Corniel, I really believe that neither of the 2 would be Pure, correct? as a requirement would only be local variables.
There was a problem hiding this comment.
The question is always: If I do not use the result of the call, is the call still useful? Of not, the method is pure.
|
@Corniel do you have any other considerations? |
|
After further inspection, I wondered: why caching the lines? If the lines don't change, the Info does not change either. As the DotNetInfo is immutable, I would argue that the result of the parsing should be cached, not the input. |
|
@Corniel thanks, it would really make sense to cache the output, I'll make the change. |
| @@ -0,0 +1,30 @@ | |||
| using Microsoft.Build.Framework; | |||
|
|
|||
| namespace Buildalyzer.Caching | |||
There was a problem hiding this comment.
Don't forget to use a files coped namespace.
Feature:
Cache dotnet --info result per project
Description
In some scenarios, executing dotnet --info may take longer than desired, especially when executed within containers with few resources.
This pr adds a cache where the key is the full path of the project.