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

import types #2192

Merged
merged 8 commits into from
Apr 3, 2024
Merged

import types #2192

merged 8 commits into from
Apr 3, 2024

Conversation

turadg
Copy link
Member

@turadg turadg commented Mar 29, 2024

Description

Uses @import from upcoming TS 5.5, like Agoric/agoric-sdk#9149

Also includes the changes needed in Agoric/agoric-sdk@83db366

Security Considerations

Nightly build of TypeScript

Scaling Considerations

no runtime changes

Documentation Considerations

nothing to change

Testing Considerations

CI

Compatibility Considerations

Increases compatibility with agoric-sdk

Upgrade Considerations

no runtime changes

  • Includes *BREAKING*: in the commit message with migration instructions for any breaking change.
  • Updates NEWS.md for user-facing changes.

Copy link
Member

@kriskowal kriskowal left a comment

Choose a reason for hiding this comment

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

Ecstatic. I think I found the CI failure. Your changes were mechanical, but there was an existing type reference error that had gone unnoticed somehow. I changed one imported type from Remotable to RemotableObject and appended that commit to your branch (1faea7a). Paging @erights if that’s consistent with the design.

@erights
Copy link
Contributor

erights commented Apr 3, 2024

Ecstatic. I think I found the CI failure. Your changes were mechanical, but there was an existing type reference error that had gone unnoticed somehow. I changed one imported type from Remotable to RemotableObject and appended that commit to your branch (1faea7a). Paging @erights if that’s consistent with the design.

Yes. Glad to see this happening!

@erights
Copy link
Contributor

erights commented Apr 3, 2024

windows-latest failed. But it is known to be flaky so I restarted it.

Copy link
Contributor

@erights erights left a comment

Choose a reason for hiding this comment

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

Not a reviewer, but joining in out of enthusiasm ;)

@turadg turadg marked this pull request as ready for review April 3, 2024 15:59
@turadg
Copy link
Member Author

turadg commented Apr 3, 2024

I changed one imported type from Remotable to RemotableObject and appended that commit to your branch (1faea7a)

Thanks. I squashed that into the commit that has the others.

I also added the patch that agoric-sdk needed. Since you reviewed that in Agoric/agoric-sdk#9185 @kriskowal I'm not requesting re-review before merging.

@turadg turadg enabled auto-merge (rebase) April 3, 2024 16:00
@erights
Copy link
Contributor

erights commented Apr 3, 2024

Just reran the flaky windows-latest again

@turadg turadg merged commit c5ba502 into master Apr 3, 2024
18 checks passed
@turadg turadg deleted the ta/endo-types branch April 3, 2024 16:55
turadg added a commit that referenced this pull request Apr 17, 2024
## Description

Follow up #2192

I think this wraps up the transition, save for the TODO.

### Security Considerations

n/a, typedefs

### Scaling Considerations

n/a, typedefs

### Documentation Considerations

not necessary

### Testing Considerations

CI
### Compatibility Considerations

Some reduction in exports, but those were deep imports so ok to break

### Upgrade Considerations

none
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