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.

Sign up for GitHub

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

Jump to bottom

fix: include inherited methods in missing method error #1382

Merged
merged 1 commit into from
Dec 4, 2022
Merged

fix: include inherited methods in missing method error #1382

merged 1 commit into from
Dec 4, 2022

Conversation

erights
Copy link
Contributor

@erights erights commented Dec 1, 2022

@erights erights self-assigned this Dec 1, 2022
@erights erights mentioned this pull request Dec 1, 2022
Merged
Chris-Hibbert
Chris-Hibbert approved these changes Dec 1, 2022
View reviewed changes
Copy link
Contributor

@Chris-Hibbert Chris-Hibbert left a comment

Choose a reason for hiding this comment

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

The test shows the outcome I'm interested in. Thanks!

I don't know enough to approve the implementation. It looks fine to me, but I'm not aware of the constraints you need to maintain here, so someone more familiar with eventual-send should take a look.

@erights
Copy link
Contributor Author

erights commented Dec 1, 2022

The test shows the outcome I'm interested in. Thanks!

I don't know enough to approve the implementation. It looks fine to me, but I'm not aware of the constraints you need to maintain here, so someone more familiar with eventual-send should take a look.

@michaelfig I think that's you. I'll hold off merging until I get your review. Thanks.

@erights erights force-pushed the 6570-markm-missing-methods-error branch from 0b69fe9 to f91d9c4 Compare December 2, 2022 03:16
@erights
Copy link
Contributor Author

erights commented Dec 2, 2022 •
edited
Loading

Hi @kriskowal , I need your help to debug https://github.com/endojs/endo/actions/runs/3596352943/jobs/6056920559 . I tried to run it locally. But simply doing yarn test:pre-release gave
>>> Failed to load resource: net::ERR_FILE_NOT_FOUND

So I then tried to puzzle my way through https://github.com/endojs/endo/blob/master/packages/ses-integration-test/README.md but immediately got stuck on the non-existence of ./scripts/build-pre-release-test.sh.

Trying to puzzle out what to do instead, I found I was out of my depth.

@kriskowal found that this error even occurs on master, and so disabled the test for now. As a result, it is no longer an impediment for this PR

@erights erights mentioned this pull request Dec 2, 2022
Merged
@erights erights force-pushed the 6570-markm-missing-methods-error branch from f91d9c4 to e7b9bfe Compare December 2, 2022 22:03
@erights erights force-pushed the 6570-markm-missing-methods-error branch from e7b9bfe to 9a4568e Compare December 2, 2022 22:09
michaelfig
michaelfig approved these changes Dec 4, 2022
View reviewed changes
Copy link
Member

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

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

LGTM!

@erights erights merged commit f0cd522 into master Dec 4, 2022
@erights erights deleted the 6570-markm-missing-methods-error branch December 4, 2022 23:55
@erights erights mentioned this pull request Jan 30, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Chris-Hibbert Chris-Hibbert Chris-Hibbert approved these changes

@michaelfig michaelfig michaelfig approved these changes

Assignees

@erights erights

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

empty diagnostic method list from amm public facet etc.
3 participants
@erights @michaelfig @Chris-Hibbert