-
Notifications
You must be signed in to change notification settings - Fork 208
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
fix(ertp): remove unneeded ertp type imports #10467
Conversation
Deploying agoric-sdk with Cloudflare Pages
|
* Importing Baggage from `@agoric/ertp` is deprecated. Import Baggage from | ||
* `@agoric/vat-data` instead | ||
* | ||
* @import {Baggage} from '@agoric/vat-data' |
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.
The only questionable one. See the discussion under "Upgrade Considerations" in the PR comment.
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.
What's the harm in leaving around extra type imports?
Distraction. Makes it a bit harder to find the type imports that are actually used. Also, makes it a bit harder to search for "who meaningfully imports this type?". Same rationale for avoiding unused imports and other dead notation in general, though clearly of weaker magnitude for type imports. |
closes: #XXXX
refs: #XXXX
Description
For some reason, vscode does successfully highlight unused jsdoc type imports
but
yarn lint
does notThis PR started as a drive-by extracted from #10456 , where I noticed that ERTP in particular had some of these. Since #10456 experiments with a variation on ERTP, simplifying it first in harmless ways helps a bit.
Security Considerations
fewer distractions helps
Scaling Considerations
none
Documentation Considerations
none
Testing Considerations
none
Upgrade Considerations
In the agoric-sdk repo there are no remaining type imports of the legacy type reexport of
Baggage
from ertp. But if there are such imports in other repos, they will need to be fixed. But this is a static-only issue, not a runtime issue, so it's fine for those repos to be fixed only once they depend on the new@agoric/ertp
without that legacy reexport.