Skip to content

Breaking-change prototype: compare records and normalize paths safely - #5755

Open
catalinaperalta wants to merge 1 commit into
Azure:feat/breaking-change-tool-upstream-draftfrom
catalinaperalta:fix/breaking-change-records-paths
Open

catalinaperalta wants to merge 1 commit into
Azure:feat/breaking-change-tool-upstream-draftfrom
catalinaperalta:fix/breaking-change-records-paths

Conversation

@catalinaperalta

Copy link
Copy Markdown
Member

Focused fix for #5450. Compares record indexer values and indexed/non-indexed model shapes, distinguishes nested generic instantiations during cycle detection, and preserves the owning property for approvals and source locations. Replaces the path regex with a monotonic scan that preserves embedded placeholders and malformed-path behavior. Addresses discussion_r3984280196, discussion_r3993294760 and discussion_r4099933236 on #5450. Build and 80 targeted engine/identity/record tests pass. Ran pnpm format and pnpm lint; repository lint remains blocked by existing prototype unused declarations, console usage and missing error cause. Unrelated formatting excluded.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: eddbfa67-cd77-4acf-99f6-8fd275d6c7d6

@markcowl Mark Cowlishaw (markcowl) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, one suggestion


describe("record comparisons", () => {
it.each([
["Record<string>", "Record<int32>", "ResourcePropertyTypeChanged"],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we do the same for Record<string>, Array<string>? Also, do we have a similar test for Array<string>, Array<string> and Array<string>, string[]?

This branch has not been deployed

No deployments
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.

3 participants