-
Notifications
You must be signed in to change notification settings - Fork 209
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 APIs for using analyzer element model 2. #3775
Conversation
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. |
How should we deal with #3774 here Merge the effort? |
I would really like to avoid propagating all the numbered variants of the APIs here, it implies a future breaking change to remove them, and build doesn't do many breaking changes (not since March 2021). What is the plan for getting rid of the old APIs here? Or do we plan to keep them around for some time? |
cc @natebosch in case you have any ideas for how to mitigate the maintenance/versioning costs here... |
I can't think of a way around a breaking version change here. I'll spend some more time thinking of how we can most smoothly roll this out, but my initial thought is that it might bet better to separate the interface at a higher level than per-method. We should consider adding a |
Note, this change is a split-out of https://critique.corp.google.com/cl/698854756 where I migrated many clients of The old APIs in the analyzer will be removed after all the clients migrated. |
In most cases, we would just do breaking changes externally (without publishing), then merge it into google3 (fixing up clients) before publishing. Externally we just expect version solving to solve this problem and don't expend a lot of effort to allow incremental adoption of new APIs. This is harder though, when the change needs to trickle through many packages, such as the analyzer case. However, I think it is probably feasible in this case. We still end up needing a breaking change it sounds like, but we don't need to introduce the new variant and then delete the old one, we can just do a single atomic swap of the types. We would update the internal packages to be compatible with the new APIs and land that at the same time as rolling in the new package:build APIs. Then once that succeeds, we publish package:build. |
@scheglov let me know if you want to just have a meeting to discuss |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed with @scheglov , we plan to have these apis temporarily just for the internal migration, but never to publish them (if we can avoid it). We will do an immediate swap to the new APIs as a breaking change, once the migration is complete.
This reverts commit 1865a35.
This reverts commit fbdfb67. I have forked the repo prior to this revert here https://github.com/dart-lang/build/tree/resolver-2-methods. The plan is to use that fork internally until all builders are updated to the new APIs. Then we will do a breaking change in this repo to migrate to all the new APIs. We do not plan to publish ever the *2 APIs on pub. In the short term, we will update our analyzer upper constraints across all packages but not add these new methods or expose the new APIs. Those versions will be published (soon ish) to unblock other work (such as updating to the latest dart_style).
See https://docs.google.com/presentation/d/1Vxr-y5ljuo0Os78R4tKZg8cc0In984EQcLs06HOqS28/edit#slide=id.p for the new element model details. It is possible that you will not able access it, unless you work for Google.
This change is green in google3.
It looks that there are issues with dependencies and versions, and possible circular or chained dependencies.
And I don't know how to deal with these issues.
However google3 uses the version of
analyzer
that is newer than7.0.0
, so it is possible that7.0.0
is not enough.But because the the version issues mentioned above, I don't see, don't get to the place where it can be checked yet.
I have analyzer 7.1.0 publishing in flight, in case it is will be necessary.
Contribution guidelines:
dart format
.Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.