Fix version inconsistency handling for PackageReference Update items - #128
Conversation
…ta was reported as fixable but could not be safely resolved
![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: 6dd73a098c
ℹ️ 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".
…— unconditional Update ordering was losing target-specific metadata
💡 Codex ReviewCPMigrate/CPMigrate/Services/ProjectFileScanner.cs Lines 480 to 482 in 4d08e11  Retain the unconditional version beside a surviving override
When an inherited reference is amended first by a conditional ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…h conditionality semantics
|
Addressed the prior P1 in 4d08e11 and finalized in 57074fd: unconditional version-bearing Updates now amend every earlier matching item while preserving conditional Include state and target-specific VersionOverride metadata. Added scanner/analyzer regressions. The prior thread is resolved; final-head CI and hosted Codex review are pending. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57074fd72f
ℹ️ 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".
| IsConditional = | ||
| existing.IsConditional | ||
| && (!existing.IsConditionalUpdate || conditionalMetadataSurvives), |
There was a problem hiding this comment.
When an inherited PackageReference is amended by a conditional Update VersionOverride="9.0.0" and then by an unconditional Update Version="3.0.0", these lines retain the only synthetic declaration as conditional because the override survives. With no local Include, there is consequently no unconditional declaration for 3.0.0; IsConditionallyDeclared treats both the ordinary resolved version and the conditional override as conditional and filters them from version-inconsistency analysis, hiding real cross-project drift. Preserve the conditional override as a separate record while also representing the later unconditional Update.
Useful? React with 👍 / 👎.
…erited Update metadata was hiding base drift
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffc6d8fad5
ℹ️ 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".
| if ( | ||
| isUpdate | ||
| && !hasPriorReference | ||
| && HasLaterInclude(items, itemIndex, packageName) | ||
| ) |
There was a problem hiding this comment.
When the package was already included by an SDK or Directory.Build.props, an Update here amends that inherited item even if this project contains a later unconditional Include; the later declaration does not make the earlier update inert. This check therefore drops a real inherited version (and IsInertUpdateBeforeInclude makes the fixer skip the same XML), so a project with an inherited Serilog, then Update="Serilog" Version="2.0.0", then a local Include="Serilog" Version="1.0.0" can have its conflict hidden or only the local declaration rewritten.
Useful? React with 👍 / 👎.
A later local Include cannot prove that an earlier Update is inert because SDK and Directory.Build.props items may already exist. Keep and amend those Update items so inherited version conflicts remain visible and fixable.
|
@codex review Please review the final head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3603e67bab
ℹ️ 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".
| if (!isUpdate || isConditional) | ||
| { | ||
| return []; |
There was a problem hiding this comment.
When two Update items execute under the same condition, the later one amends the metadata set by the earlier one, but this unconditional return keeps both declarations. For example, conditional Update="Pkg" Version="4.*" followed under the identical condition by Update="Pkg" Version="4.0.0" is exact after evaluation, while FloatingVersionAnalyzer still sees the superseded 4.* record and reports a false floating-version finding. Preserve separate records for distinct conditional branches, but fold sequential updates with the same effective condition.
Useful? React with 👍 / 👎.
A later Update in the same effective condition amends the earlier item, so retaining the superseded floating specification creates a false analyzer finding. Track conditional scope and fold only updates in the same branch while preserving distinct conditions.
|
@codex review Please review the new final head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19f5c5c243
ℹ️ 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".
| if (current is ProjectOtherwiseElement) | ||
| { | ||
| return true; | ||
| conditions.Add("<Otherwise>"); |
There was a problem hiding this comment.
Using the constant "<Otherwise>" makes every Otherwise block share a scope, even across different Choose elements whose sibling When predicates differ. If the first Otherwise contains Update="Pkg" Version="4.*" and a later, independently controlled Otherwise contains version 4.0.0, only the first may execute in a given evaluation, but FindAmendmentIndices folds them and hides the active floating version. Include the enclosing Choose branch identity or its effective predicate so independently evaluated branches remain separate.
Useful? React with 👍 / 👎.
Otherwise has no condition text of its own, so a constant scope merged branches from separate Choose elements and hid active version evidence. Include the structural branch path so only the same conditional branch can fold sequential Updates.
|
@codex review Please review final head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 796d6f90c5
ℹ️ 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".
| item.existing.IsConditionalUpdate | ||
| && string.Equals( | ||
| item.existing.ConditionalScope, | ||
| conditionalScope, | ||
| StringComparison.Ordinal |
There was a problem hiding this comment.
When an Include and a later Update are inside the same conditioned ItemGroup or When branch, MSBuild applies the update to that included item. This predicate only matches earlier records marked IsConditionalUpdate—and conditional includes do not retain their scope—so a conditional Include Version="4.*" followed by Update Version="4.0.0" remains as two declarations and FloatingVersionAnalyzer falsely reports the superseded 4.*. Match conditional item declarations in the same structural scope while continuing to keep independent branches separate.
Useful? React with 👍 / 👎.
A conditional Update in the same item scope amends a conditional Include just like a prior conditional Update. Retain the branch identity on Includes so stale floating versions are not reported while independent branches remain visible.
|
@codex review Please review final head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35ef744bb8
ℹ️ 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".
| && ( | ||
| !isConditional | ||
| || ( | ||
| item.existing.IsConditional | ||
| && string.Equals( | ||
| item.existing.ConditionalScope, | ||
| conditionalScope, | ||
| StringComparison.Ordinal | ||
| ) | ||
| ) |
There was a problem hiding this comment.
When an unconditional reference already has VersionOverride and a later conditional Update sets only Version (for example, Version="4.*"), MSBuild retains the existing override, so the update does not make 4.* effective. This predicate refuses to amend the unconditional record and instead emits a standalone conditional record without the inherited override; FloatingVersionAnalyzer then reads its Version and reports a false floating-version finding. Retain the unconditional base record, but synthesize the conditional effective record with metadata not replaced by the update.
Useful? React with 👍 / 👎.
|
@codex review Please review exact final head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 858d091942
ℹ️ 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".
| && string.IsNullOrWhiteSpace(versionOverride) | ||
| ? references | ||
| .LastOrDefault(existing => | ||
| !existing.IsConditional |
There was a problem hiding this comment.
When a base Update VersionOverride="1.0.0" is followed under condition C by Update VersionOverride="2.*" and then a same-scope Update Version="3.0.0", MSBuild leaves 2.* in force because the last update does not replace VersionOverride. This lookup considers only unconditional records, so the final update passes 1.0.0 to ApplyAmendments and overwrites the nearer conditional 2.*, causing FloatingVersion to miss the active float. The fresh final-head case is the additional same-scope conditional override between the base and Version-only update; prefer that metadata over the unconditional base.
Useful? React with 👍 / 👎.
|
@codex review Please review exact final head |
|
Correction to the previous review request: the exact pushed head is |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4de9613232
ℹ️ 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".
| if (string.IsNullOrWhiteSpace(version)) | ||
| { | ||
| hasActiveClear = hasInlineVersion; | ||
| } |
There was a problem hiding this comment.
When the package is introduced by an earlier import and the project contains only an unconditional <PackageReference Update="Foo" Version="" />, this initializes hasInlineVersion to false and therefore does not recognize the Update as an active clear. The safety preflight then passes, but UpdateProjectVersions skips the empty metadata and can report that no fix is needed while the imported version remains cleared and the effective central version still conflicts. Fresh evidence at this head is the new guard requiring a locally observed nonempty Version before treating a clear as unsafe; Update-only histories need to start conservatively because imported item metadata is not visible in this XML.
Useful? React with 👍 / 👎.
|
@codex review Please review the exact current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 975785c084
ℹ️ 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".
| // An Update-only history may target an item imported from Directory.Build.props or an SDK. | ||
| // Its inherited VersionOverride is not visible in this local XML, so begin conservatively as | ||
| // potentially overridden until a local declaration proves otherwise. | ||
| (null, 0, true, false), |
There was a problem hiding this comment.
When a local Include="Foo" Version="1.0.0" is followed by a conditional Update that clears VersionOverride and then an unconditional Update that restates Version="1.0.0", this sentinel remains active even though the local Include has no override. The conditional clear marks the sentinel, causing HasConditionalOverrideClearAfter to skip rewriting the final, effective Version Update; the fixer rewrites only the superseded Include and reports success while the project still resolves the old version. Remove or otherwise neutralize the speculative inherited state once a local item history is established, or fail the fix instead of reporting a partial rewrite as successful.
Useful? React with 👍 / 👎.
| if (hasConditionalVersionClear) | ||
| { | ||
| return true; |
There was a problem hiding this comment.
When a project has an unconditional Version="1.0.0" plus a conditional Version="" Update that exposes central version 2.0.0, this package-wide return classifies both resolved versions as conditional. Protecting 2.0.0 is correct, but the unconditional 1.0.0 is also removed from version-inconsistency analysis, so drift between that branch and another project's pin is missed. At this head the new hasConditionalVersionClear result is not tied to the version argument; preserve protection only for the resolved central branch rather than every reference from the project.
Useful? React with 👍 / 👎.
| private static bool IsNonLiteralVersion(string version) | ||
| { | ||
| return !string.IsNullOrWhiteSpace(version) | ||
| && ( | ||
| version.Contains("$(", StringComparison.Ordinal) | ||
| || !NuGetVersion.TryParse(version, out _) | ||
| ); |
There was a problem hiding this comment.
When a conditional declaration uses an exact NuGet range such as [1.0.0] (or [1.0.0,1.0.0]) alongside an unconditional 1.0.0 pin, NuGetVersion.TryParse rejects the range syntax and this method labels it non-literal. The early conditional protection then excludes the project's unconditional resolved version from inconsistency analysis, potentially hiding drift against another project, even though these ranges deterministically pin one release. Parse ranges with VersionRange and treat equal inclusive bounds as literal, consistent with FloatingVersionAnalyzer.
Useful? React with 👍 / 👎.
|
@codex review Please review the exact current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ec5c58a28
ℹ️ 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".
| return !NuGetVersion.TryParse(version, out _) | ||
| && !IsExactVersionRange(version); |
There was a problem hiding this comment.
When a project has an unconditional 1.0.0 declaration and a conditional exact range [2.0.0], the resolved 2.0.0 branch is no longer classified as non-literal here, but matching still compares the declaration text [2.0.0] directly with the resolved text 2.0.0. Because the project also has an unconditional declaration, the final fallback returns false, so the conditional branch enters version-inconsistency analysis and remains after the fixer rewrites only unconditional metadata, leaving the finding unresolved. Fresh evidence beyond the earlier exact-range comment is that this head exempts exact ranges without adding semantic range-to-version matching.
Useful? React with 👍 / 👎.
|
@codex review Please review the exact current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd0ff13462
ℹ️ 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".
| if (isUpdate && !hasVersionMetadata && hasPriorReference) | ||
| { | ||
| continue; |
There was a problem hiding this comment.
When an unconditional metadata-only Update precedes a local Include, such as <PackageReference Update="Foo" PrivateAssets="all" /> followed by <PackageReference Include="Foo" />, hasPriorReference is false, so this retains the Update and later adds the Include. RedundantReferenceAnalyzer does not filter IsMetadataOnlyUpdate, so it reports two declarations even though the Update creates no item; the finding is also marked fixable, but RedundantReferenceFixer sees only the single Include and makes no change. Keep this record available for casing analysis without allowing it to count as an independent reference.
Useful? React with 👍 / 👎.
|
@codex review Please review the exact current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0d4564d07
ℹ️ 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".
| .GroupBy(NormalizeConditionSyntax, StringComparer.Ordinal) | ||
| .Select(group => group.First()) | ||
| .ToArray(); | ||
| return conditions.Length == 0 ? null : string.Join(" || ", conditions); |
There was a problem hiding this comment.
When Version is guarded by Configuration == Debug And TargetFramework == net8.0 and a floating VersionOverride is guarded by TargetFramework == net8.0, ShouldSplitConditionedMetadata keeps one projection and this serializes its scope using ||. A later exact override Update under TargetFramework == net8.0 supersedes the float everywhere it can apply, but ConditionCovers only recognizes the word Or; it treats the || expression as a conjunct and can leave the old floating projection alive, producing a false FloatingVersion finding. Fresh evidence beyond the previously fixed identical-guard case is that distinct covered guards still take this || path; serialize a recognized Or expression or preserve the guards structurally.
Useful? React with 👍 / 👎.
|
@codex review Please review the exact current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6df193017d
ℹ️ 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".
|
|
||
| foreach (var reference in matchingReferences) | ||
| { | ||
| foreach (var version in GetUnconditionalMetadataValues(reference, "Version")) |
There was a problem hiding this comment.
When an unconditional VersionOverride="" Update clears an earlier override, this preflight examines only ordinary Version metadata and therefore permits the fix. For example, an Include with VersionOverride="1.0.0" followed by an unconditional override clear can resolve to central version 2.0.0; fixing drift to 3.0.0 rewrites the superseded 1.0.0 while leaving the clear active, then reports a successful change although the effective version remains 2.0.0. Unlike the handled Version-clear case, the exact head never checks VersionOverride clears here.
Useful? React with 👍 / 👎.
| !string.IsNullOrWhiteSpace( | ||
| GetMetadataValues(element, "VersionOverride").LastOrDefault() | ||
| ), |
There was a problem hiding this comment.
When an Include has an unconditional Version="1.0.0" and a conditioned VersionOverride="2.0.0", this initializes the override as globally active. A later conditional override clear in a mutually exclusive branch then causes HasConditionalOverrideClearAfter to protect the ordinary version, even though that clear cannot clear the conditioned override. If the unconditional branch is reported drifting to 3.0.0, the fixer skips its Version, modifies nothing in this project, and can return NoFixNeeded while the conflict remains; track the override and clear scopes rather than reading all metadata as unconditional state.
Useful? React with 👍 / 👎.
| var closingIndex = conditionalScope.IndexOf(')', index + 2); | ||
| if (closingIndex <= index + 2) |
There was a problem hiding this comment.
When identical package guards use a property function such as $([System.String]::Equals('$(Mode)', 'A')), this lookup takes the first closing parenthesis, treats the whole prefix through $(Mode) as an invalid property name, and then advances past the nested property without recording Mode. If Mode is reassigned between a floating Include and an exact Update, PropertyMutationStatesMatch consequently folds them even though the Update no longer executes, hiding the effective floating version. The condition-property extraction needs to recognize nested property references rather than skipping them.
Useful? React with 👍 / 👎.
|
@codex review Please review the exact current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7449e5af7
ℹ️ 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".
| isUpdate | ||
| && hasVersionMetadata | ||
| && IsUnevaluablePackageSpecification(packageName) |
There was a problem hiding this comment.
When an Include uses an evaluated specification such as <PackageReference Include="$(PackageId)" Version="1.0.0" />, this guard applies only to Updates, so the declaration scan records a fictitious package named $(PackageId) while the resolved scan reports the actual package. A version-inconsistency finding for the resolved package is then marked fixable, but VersionInconsistencyFixer cannot match the expression-valued Include and reports a no-op. Fail the declared scan for unevaluable version-bearing Includes as well, or expand them before analysis.
Useful? React with 👍 / 👎.
| if (isUpdate && !hasVersionMetadata && hasPriorReference) | ||
| { | ||
| continue; |
There was a problem hiding this comment.
When <PackageReference Include="Foo" /> is followed by <PackageReference Update="foo" PrivateAssets="all" />, this branch discards the Update because a prior reference exists. DuplicatePackageAnalyzer therefore cannot detect the inconsistent casing even though its fixer supports Update attributes; the newly added IsMetadataOnlyUpdate filter in RedundantReferenceAnalyzer already prevents this record from creating a false duplicate. Retain the marked metadata-only record regardless of declaration order.
Useful? React with 👍 / 👎.
|
|
||
| private static IEnumerable<string> ExpandPackageNames(string packageName) | ||
| { | ||
| return packageName.Split(';', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); |
There was a problem hiding this comment.
Expanding list-valued declarations exposes casing findings that DuplicatePackageFixer cannot repair token-by-token. For example, Include="foo;Bar" followed by two Include="Foo" declarations makes Foo the selected standard casing, but the fixer compares the entire foo;Bar attribute with foo, finds nothing to rewrite, and returns NoFixNeeded while the finding remains. Either teach the casing fixer to replace individual list entries while preserving siblings or exclude these declarations from fixable casing findings.
Useful? React with 👍 / 👎.
|
@codex review Please review the exact current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c69d0b51dc
ℹ️ 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".
| && item.reference.HasVersionMetadata | ||
| && string.IsNullOrWhiteSpace(item.reference.Version) | ||
| && item.reference.VersionOverride is null |
There was a problem hiding this comment.
When a centrally managed reference has an unconditional VersionOverride="2.0.0" and a conditional Update clears it with VersionOverride="", the cleared branch resolves to the central version even though no declaration text names that version. This protection only recognizes Version clears, so IsConditionallyDeclared classifies the central branch as unconditional; version-inconsistency fixing can then rewrite the unconditional override while leaving the conditional branch on the old central pin, and the reported conflict survives. Detect conditional HasVersionOverrideMetadata clears analogously.
Useful? React with 👍 / 👎.
…oid false cross-project drift
|
@codex review Please review the exact current PR head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c54368d30e
ℹ️ 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".
| item.reference.IsConditionalUpdate | ||
| && hasMetadata(item.reference) | ||
| && isClear(item.reference) |
There was a problem hiding this comment.
The generic helper never checks that getValue(item.reference) is empty, and both new isClear delegates inspect only the sibling metadata. Consequently, nonempty conditional assignments are treated as clears. For example, a versionless central Include followed by conditional Version="1.0.0" and Version="2.0.0" Updates makes the central version used outside those conditions appear conditionally declared, so VersionInconsistencyAnalyzer suppresses legitimate drift for that central branch. Restore the emptiness check for the metadata being classified as cleared.
Useful? React with 👍 / 👎.
…id suppressing drift
|
@codex review Please review the exact current PR head |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
Verification