refactor: postponed removals and simplifications#8771
Conversation
3ec1016 to
1b5fbf7
Compare
|
@turadg , at https://github.com/Agoric/agoric-sdk/actions/runs/7591791789/job/20680380230?pr=8771#step:4:361 I'm getting one block of type errors I'd like your help on. These errors only happen indirectly in the @agoric/casting package. There are no such errors in the packages these things are imported from. Nor in any other packages that import them. What's special about @agoric/casting? |
|
At endojs/endo#1933 (comment) , I ask about possible interaction with some endo side changes that were also waiting until after the endo-agoric-sdk sync. |
Nothing particularly special. Each package is a separate TypeScript project so they each resolve differently (see #4404 as a spike to unify). In this case We rely on searching because of the decision years ago to rely on ambient types. The more we use type modules (with imports and exports) the less we'll have this problem. One reason it's happening in your PR is that making The change also reduces the maxNodeModuleJsDepth to 1, shrinking the type import graph. That caused another problem, with ThisType in Separately, your "cleanups in this PR" list has several items but so far it's all in one commit. Please consider separating the changes by commit. It will be easier to review that way. |
6e0290e to
ac7afd2
Compare
The vast majority of the bulk were changing imports, and use of the |
Chris-Hibbert
left a comment
There was a problem hiding this comment.
This looks fine to me.
My impression of the current intended process is that we can merge changes to master as they get approved, and the hard version compatibility questions will be answered as we decide when to migrate to a release branch. @warner, is that a correct reading of the theory?
a49db10 to
4a722f8
Compare
ac7afd2 to
1c8af6d
Compare
f4b29d2 to
b5120d8
Compare
b5120d8 to
d59dc41
Compare
|
@Mergifyio requeue |
✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically |
Staged on #8781
closes: #XXXX
refs: #8749 endojs/endo#1985 #8781
Description
Now that a recent endo is released and agoric-sdk has been upgraded to depend on it, we can finally get to a lot of agoric-sdk technical-debt cleanups that we've postponed. The cleanups in this PR include:
@agoric/same-structurepackage, that had no remaining users anyway.Security Considerations
Less code helps security. No other effect on security
Scaling Considerations
none. (or minor help due to smaller total code)
Documentation Considerations
Once these reductions are complete and we can delete legacy exporters, there will be less to document.
Testing Considerations
Of the redundant code copies here, the tests here a bit bigger than the ones in their new home. So endojs/endo#1985 will preserve this extra test code.
Upgrade Considerations
@ivanlei added you as a reviewer for advice on when it would not be too disruptive to merge this.