feat: support combined anonymous instance identifier format#61
feat: support combined anonymous instance identifier format#61Artifizer merged 1 commit intoGlobalTypeSystem:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds UUID-tail support for GTS IDs: a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
18cc42a to
8daeff8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gts-id/src/lib.rs`:
- Around line 818-833: Run rustfmt on the repository and apply formatting fixes
to the test block around the function
test_uuid_tail_without_preceding_tilde_rejected in gts-id/src/lib.rs; ensure the
test formatting (indentation, spacing, and trailing commas) matches cargo fmt
expectations and re-run cargo fmt so the call to validate_gts_id and the match
on GtsIdError::Id are consistently formatted per rustfmt.
In `@gts/src/gts.rs`:
- Around line 148-158: match_segments currently treats two UUID-tail segments as
equal by comparing only parsed fields (vendor, package, namespace, type_name,
ver_major, ver_minor, is_type), which are empty for UUID tails; update
match_segments so that when both Segment::is_uuid_tail is true and the pattern
segment does not contain a wildcard ('*'), you first compare the raw
Segment::segment string (the actual UUID) for equality before falling back to
parsed-field comparisons; modify the branch that handles UUID-tail segments in
match_segments to perform this explicit segment-string check to ensure
non-wildcard UUID-tail matching requires the same UUID.
Signed-off-by: Matt Garmon <matt.garmon@acronis.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
gts/src/gts.rs (1)
263-266: UUID tail matching works correctly, though could be more explicit.The current implementation addresses the past review concern about different UUIDs comparing equal. The logic is functionally correct because:
- If pattern is UUID tail and segments differ → returns false
- If pattern is UUID tail and segments match → continues to empty field comparisons which pass
- If candidate is UUID tail but pattern isn't → vendor/package checks fail
However, the suggested fix from the past review was more explicit:
if p_seg.is_uuid_tail || c_seg.is_uuid_tail { if p_seg.is_uuid_tail != c_seg.is_uuid_tail || p_seg.segment != c_seg.segment { return false; } continue; }This would:
- Explicitly reject mismatched UUID-tail flags
- Skip unnecessary empty-string field comparisons via
continueThe current code works, but consider adopting the more explicit form for clarity and slight efficiency gain.
♻️ Optional: More explicit UUID tail handling
- // Non-wildcard UUID tail - compare raw segment string (the actual UUID) - if p_seg.is_uuid_tail && p_seg.segment != c_seg.segment { - return false; - } + // UUID tail segments must match exactly (both must be UUID tails with same UUID) + if p_seg.is_uuid_tail || c_seg.is_uuid_tail { + if p_seg.is_uuid_tail != c_seg.is_uuid_tail || p_seg.segment != c_seg.segment { + return false; + } + continue; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gts/src/gts.rs` around lines 263 - 266, Replace the implicit UUID-tail handling with an explicit branch: when either p_seg.is_uuid_tail or c_seg.is_uuid_tail is true, first require they match (p_seg.is_uuid_tail == c_seg.is_uuid_tail) and then require the raw segment strings to match (p_seg.segment == c_seg.segment); if either check fails return false, otherwise continue the loop to skip the subsequent empty-field comparisons. Target the loop that inspects p_seg and c_seg (using the is_uuid_tail and segment fields) and replace the current single-case check with this explicit two-step check and a continue on success.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@gts/src/gts.rs`:
- Around line 263-266: Replace the implicit UUID-tail handling with an explicit
branch: when either p_seg.is_uuid_tail or c_seg.is_uuid_tail is true, first
require they match (p_seg.is_uuid_tail == c_seg.is_uuid_tail) and then require
the raw segment strings to match (p_seg.segment == c_seg.segment); if either
check fails return false, otherwise continue the loop to skip the subsequent
empty-field comparisons. Target the loop that inspects p_seg and c_seg (using
the is_uuid_tail and segment fields) and replace the current single-case check
with this explicit two-step check and a continue on success.
Summary by CodeRabbit
New Features
Tests
Chores