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

Endo integration scratch PR #9385

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Endo integration scratch PR #9385

wants to merge 1 commit into from

Conversation

turadg
Copy link
Member

@turadg turadg commented May 20, 2024

#endo-branch: master

Description

Persistent scratch PR for testing Endo integration, using

} else if (context.payload.pull_request) {
const { body } = context.payload.pull_request;
const regex = /^\#endo-branch:\s+(\S+)/m;
const result = regex.exec(body);
if (result) {

#9285 is an example of how it works. This PR should never leave draft because it's never meant to merge, just test. It should never be closed, so that we can use it again and again for different Endo branches.

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

turadg added a commit to endojs/endo that referenced this pull request May 21, 2024
refs: #1488 

## Description

Simplify build by using NPM lifecycle hooks. Adds a CI test for the
package graph sensitivity that the extant method was checking.

Part of making it work was adding "typescript" to `devDependencies` of
packages that need it. The root `build:types` was running with the root
`tsc` but when building in each package separately they wouldn't get the
root's typescript version and fall back to the global one, which didn't
know the `@import` syntax.


### Security Considerations

None

### Scaling Considerations

None

### Documentation Considerations

Less to document 

### Testing Considerations

I tried the "publish" commands in CONTRIBUTING.md. 

integration PR in agoric-sdk:
Agoric/agoric-sdk#9385

### Compatibility Considerations

Removes a necessary accommodation on agoric-sdk

### Upgrade Considerations

None
Copy link

cloudflare-workers-and-pages bot commented May 21, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: a347ffb
Status: ✅  Deploy successful!
Preview URL: https://4cc9a869.agoric-sdk.pages.dev
Branch Preview URL: https://endo-integration-temp.agoric-sdk.pages.dev

View logs

@kriskowal kriskowal added the force:integration Force integration tests to run on PR label May 21, 2024
Comment on lines 98 to 97
# Convention used in Endo
yarn lerna run clean:types
Copy link
Member Author

Choose a reason for hiding this comment

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

mergify bot added a commit that referenced this pull request Aug 21, 2024
#endo-branch: master

## Description

Removes some build code that references Endo code that's no longer there.

Also slips in a typedefs fix to re-spin CI after I changed the endo-branch target. These fixes are good to get in ASAP and didn't seem worth a whole other PR.

After this merges I think #9385 can be reset to master

### Security Considerations

none

### Scaling Considerations

none

### Documentation Considerations

none

### Testing Considerations

CI suffices

### Upgrade Considerations

none
@turadg turadg closed this Aug 21, 2024
@turadg
Copy link
Member Author

turadg commented Aug 21, 2024

Accidentally closed by a PR comment

turadg added a commit to endojs/endo that referenced this pull request Oct 18, 2024
_incidental_

## Description

In an a3p test I tried disabling `skipLibCheck` and many errors were
from Endo packages's omission of module filename extensions.

That's because Endo was using the default
[moduleResolution](https://www.typescriptlang.org/tsconfig/#moduleResolution),
`node10`. This changes it to `NodeNext` and also the `module` option for
compatibility.

That resulted in new errors which I address mostly by switching to
explicit `@import`.

### Security Considerations

none

### Scaling Considerations

none

### Documentation Considerations

none

### Testing Considerations

- Agoric/agoric-sdk#9385

### Compatibility Considerations

This might affect consuming packages that depend on ambient types.
That's almost certainly only agoric-sdk which we can solve with
Agoric/agoric-sdk#9385

### Upgrade Considerations

If Agoric/agoric-sdk#9385 needs changes then
`NEWS.md` should indicate it. I don't think type changes live up to
`BREAKING` which would be a semver major bump.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants