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
refactor(ivy): remove ngBaseDef #33264
Conversation
aa0e499
to
bf5c2b2
Compare
bf5c2b2
to
7f5b5a6
Compare
Initial set of presubmits: |
Quick update after running presubmit in g3: it looks like there are some regressions, but they seem to be caused by the fact that there is a problem with inheriting inputs from base classes (annotated with |
Are those compile-time errors or runtime? If I remember correctly, we have a lot of inheritance tests for the runtime which are passing. |
7f5b5a6
to
ac4de4b
Compare
b87ed22
to
c2d7743
Compare
c2d7743
to
2743778
Compare
FYI, VE and Ivy presubmits were successful (g3 regression was resolved) at the point when I started them (see my comment here). I believe CI was "green" at that time as well, so I'm wondering if we can roll back recent changes (based on CR) in this PR if they are not critical, so we can unblock this PR. The remaining comments/improvements (if they are not critical) can be performed as a followup, especially given that we became blocked on #32987. |
a69d119
to
077f442
Compare
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
077f442
to
402033e
Compare
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
Caretaker: these are being done preemptively. Once #32987 lands, this should be in a good state. |
Removes `ngBaseDef` from the compiler and any runtime code that was still referring to it. In the cases where we'd previously generate a base def we now generate a definition for an abstract directive.
402033e
to
e957e63
Compare
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Removes
ngBaseDef
from the compiler and any runtime code that was still referring to it. In the cases where we'd previously generate a base def we now generate a definition for an abstract directive.