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

Introduction of strongly typed immutable representation of the dotnet --info output #252

Merged
merged 2 commits into from
Mar 2, 2024

Conversation

Corniel
Copy link
Contributor

@Corniel Corniel commented Feb 19, 2024

There have been some issues with the output of dotnet --info. I decided to have a look at what info is in there, and which info is used. I've reverse engineered what is currently used, added some fields that might be useful, and created a test.

This data structure should give a lot of inside in what is going on. Furthermore, it should be easy to cache this result.

My first idea was that this model should be called DotNetInfo, but (Dot)NetInvironement(Info) and others might also be good candidates.

Furthermore, the paths involved are 'tricky'. I would argue that we might want to use '/' only in our paths, as Windows can deal with those easily, and UNIX/Linux has troubles the other way around.

@phmonte
Copy link
Owner

phmonte commented Feb 24, 2024

I like DotNetInfo, it's closer to dotnet --info and I also didn't find any name that could cause a wrong interpretation.

The cache is very interesting, I've had several problems with the response time of dotnet --info, especially when running on a k8s cluster with limited processing and memory.

Thank you for structuring the return.

@phmonte phmonte added the Ready to Merge Ready to merge label Feb 24, 2024
@Corniel
Copy link
Contributor Author

Corniel commented Feb 24, 2024

@phmonte I did a rebase. Where to put the cache in, and how (if at all) to make this info part of AnalyerResults I'm not sure. Do you have any preference on that?

@phmonte
Copy link
Owner

phmonte commented Feb 26, 2024

@Corniel I thought about saving the result of the dotnet --info linked to the path.

Ex:
If the dotnet --info output returns something in the "global.json file"

key: /my-project/api/project1
value: dotnet output --info

If nothing returns inside global.json file:
key: "global" or "default"?
value: dotnet output --info

If possible, I like the alternative of storing this cache in static variables, mainly because I only use this cache when executing the project, but the InMemory cache is also an option.

Does it make sense for you?

@Corniel
Copy link
Contributor Author

Corniel commented Feb 26, 2024

Where to store it and when, is a debate on its own. I think the DotNetInfo could be part of the AnalyzerResult. As a performance improvement, those results might (and now could be) cached. I would prefer to make those decisions in a separate PR.

@phmonte
Copy link
Owner

phmonte commented Feb 27, 2024

I agree, let's think about this possibility in another PR.

@phmonte phmonte merged commit df25401 into phmonte:main Mar 2, 2024
6 checks passed
@Corniel Corniel deleted the dotnet-info branch March 2, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to Merge Ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants