Compare declared target framework sets, not raw property text - #218
Conversation
FrameworkAlignmentAnalyzer grouped projects by the raw TargetFramework(s) string: 'net8.0;net10.0' and 'net10.0;net8.0' reported a divergence that does not exist, an expression like $(SharedTfms) was listed as a framework, and an unreadable project counted as 'Unknown'. Projects are now compared by their declared target set via GetDeclaredTargetFrameworks, and uninspectable files are named as unexamined. EnvironmentAnalyzer uses the same read so the interactive summary stops counting expression text as runtimes. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebaf22366c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // "net10.0;net8.0" are the same set, not a divergence — and projects whose targets cannot | ||
| // be read (unparseable file, expression-valued TFM) are unexamined rather than reported | ||
| // under a synthetic "Unknown" framework. | ||
| var configurations = new Dictionary<string, List<string>>(StringComparer.Ordinal); |
There was a problem hiding this comment.
When projects declare the same target set using different casing, such as net8.0;net9.0 and NET8.0;NET9.0, the values retain their original spelling and this ordinal dictionary treats the joined keys as separate configurations. The analyzer therefore reports a false framework divergence despite its intended case-insensitive comparison; use a case-insensitive key comparer or normalize each TFM before constructing the key.
Useful? React with 👍 / 👎.
| "Multiple Frameworks", | ||
| $"Repository uses {frameworks.Count} different Target Frameworks: {tfmList}. Ensure package versions in Directory.Packages.props are compatible with all.", | ||
| frameworks.Values.SelectMany(v => v).ToList(), | ||
| $"Repository uses {configurations.Count} different Target Frameworks: {tfmList}.{unexaminedNote} Ensure package versions in Directory.Packages.props are compatible with all.", |
There was a problem hiding this comment.
For overlapping multi-target sets—for example, one project targeting net8.0;net9.0 and another targeting net8.0;net10.0—configurations.Count is 2 while tfmList contains three distinct frameworks. The resulting message says 2 different Target Frameworks: net8.0, net9.0, net10.0, which is internally inconsistent; report distinctFrameworks.Count or describe and enumerate the two configurations instead.
Useful? React with 👍 / 👎.
1 participant
Summary
FrameworkAlignmentAnalyzergrouped projects by the rawTargetFramework(s)property string:net8.0;net10.0andnet10.0;net8.0reported a divergence that does not exist, an expression like$(SharedTfms)was listed as a framework name, and an unreadable project counted as"Unknown"— inflating the count with a runtime nothing targetsGetDeclaredTargetFrameworks(splitsTargetFrameworks, drops expressions and conditional declarations): order- and case-insensitive, with uninspectable files named as unexamined in the finding rather than inventedEnvironmentAnalyzer(the interactive environment summary) uses the same declared-framework read, so it no longer counts expression text or"Unknown"as target frameworksTest plan
Generated with Devin