Skip to content

Support direct Location assignment to variant paths#230

Closed
lubomir wants to merge 1 commit intorelease-engineering:developmentfrom
lubomir:fix-issue-227-location-assignment
Closed

Support direct Location assignment to variant paths#230
lubomir wants to merge 1 commit intorelease-engineering:developmentfrom
lubomir:fix-issue-227-location-assignment

Conversation

@lubomir
Copy link
Contributor

@lubomir lubomir commented Mar 12, 2026

When users assign a Location object to variant.paths.repository[arch], the system now properly handles it instead of wrapping it in another Location during serialization.

Changes:

  • Modified VariantPaths._serialize_v2() to detect when a value is already a Location object (from direct assignment)
  • Store the Location in _locations for v2.0 round-trip fidelity
  • Maintain backward compatibility with string assignments

Fixes: #227

When users assign a Location object to variant.paths.repository[arch],
the system now properly handles it instead of wrapping it in another
Location during serialization.

Changes:
- Modified VariantPaths._serialize_v2() to detect when a value is
  already a Location object (from direct assignment)
- Store the Location in _locations for v2.0 round-trip fidelity
- Maintain backward compatibility with string assignments

Fixes: release-engineering#227
Assisted-By: OpenCode (claude-sonnet-4-5@20250929)
@lubomir lubomir requested a review from guillermodotn March 12, 2026 10:09
@lubomir
Copy link
Contributor Author

lubomir commented Mar 12, 2026

This is marked as a draft, because I'm not sure this is the correct approach. It allows assigning Location directly into the path, but that leads to a weird situation where the variant paths return type can be a string or a Location depending on how the metadata was loaded or created. That is likely going to be confusing.

Copy link
Contributor

@guillermodotn guillermodotn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Witing to _locations during serialization makes serialize() a mutating operation, which can backfire us back.

And if the field dict and _locations both hold a Location but get out of sync, the one in the field dict always wins because isinstance is checked first.

Checking _locations first and falling back to isinstance for direct assignment fixes both.

@lubomir
Copy link
Contributor Author

lubomir commented Mar 12, 2026

Let's close this in favor of #231

@lubomir lubomir closed this Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants