Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add code for structural equality, check that keys are upgradeable (2.9.x) #19666

Merged
merged 33 commits into from
Sep 2, 2024

Conversation

dylant-da
Copy link
Contributor

@dylant-da dylant-da commented Jul 24, 2024

Builds on work from #19184 and #19509

@dylant-da dylant-da force-pushed the upgrade-checks-disallow-key-upgrades-2.9.x branch from 0212e83 to a1db246 Compare August 1, 2024 15:41
@dylant-da dylant-da changed the title Check keys for structural equality (2.9.x) Add code for structural equality, check that keys are upgradeable (2.9.x) Aug 26, 2024
Copy link
Contributor

@paulbrauner-da paulbrauner-da left a comment

Choose a reason for hiding this comment

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

Generally LGTM but I believe that the type constructor comparison may not always work as intended depending in which order we treat the dependencies.

Maybe that doesn't matter much here as we'd always recheck all packages? I'm not clear on what triggers a comparison at which level.

@dylant-da dylant-da linked an issue Aug 27, 2024 that may be closed by this pull request
dylant-da and others added 7 commits August 29, 2024 18:00
…mplate (#19817)

* Forbid adding new interface instances to the upgraded version of a template

* allow adding new instances to new templates
…ion of a template (#19820)

* [compiler] forbid adding new interface instances to the upgraded version of a template

* allow adding new instances to new templates

* add two more tests
@dylant-da
Copy link
Contributor Author

I cherry-picked changes from #19817 and #19820 since I had to make the 2.x port of this PR (#19667) compatible with those changes anyways.

@dylant-da dylant-da enabled auto-merge (squash) August 30, 2024 16:44
@dylant-da dylant-da enabled auto-merge (squash) August 30, 2024 16:49
Copy link
Contributor

@paulbrauner-da paulbrauner-da left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

, toListOf (packageRefs . _PRImport) pkg
)
fromPkgNode (x, _pkgId, _deps) = x
sccs = G.stronglyConnCompR (map toPkgNode pkgs)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe mention that stronglyConnCompR also performs a topo sort as a comment.

@dylant-da dylant-da merged commit 5afd518 into release/2.9.x Sep 2, 2024
18 checks passed
@dylant-da dylant-da deleted the upgrade-checks-disallow-key-upgrades-2.9.x branch September 2, 2024 14:48
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.

Improve testing for deps upgrade checks
2 participants