Skip to content

Sync Endo 2023-11#8514

Closed
kriskowal wants to merge 11 commits into
masterfrom
kriskowal-sync-endo-2023-11
Closed

Sync Endo 2023-11#8514
kriskowal wants to merge 11 commits into
masterfrom
kriskowal-sync-endo-2023-11

Conversation

@kriskowal

@kriskowal kriskowal commented Nov 9, 2023

Copy link
Copy Markdown
Member

No description provided.

@kriskowal kriskowal force-pushed the kriskowal-sync-endo-2023-11 branch from 436b99e to 7b43ada Compare November 11, 2023 02:08
@kriskowal kriskowal force-pushed the kriskowal-sync-endo-2023-11 branch 2 times, most recently from b212093 to bb068f6 Compare November 28, 2023 01:08
@kriskowal kriskowal force-pushed the kriskowal-sync-endo-2023-11 branch 7 times, most recently from 2d73d09 to b693ab6 Compare December 7, 2023 01:57
@mergify mergify Bot mentioned this pull request Dec 7, 2023
@kriskowal kriskowal force-pushed the kriskowal-sync-endo-2023-11 branch from b693ab6 to ee5f04b Compare December 7, 2023 02:14
@mergify

mergify Bot commented Dec 7, 2023

Copy link
Copy Markdown
Contributor

⚠️ The sha of the head commit of this PR conflicts with #8632. Mergify cannot evaluate rules on this PR. ⚠️

@kriskowal kriskowal force-pushed the kriskowal-sync-endo-2023-11 branch 7 times, most recently from 69801d4 to 21a95f0 Compare December 9, 2023 00:28

@erights erights left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving FWIW. You should probably still wait for another reviewer.

Comment thread packages/swingset-xsnap-supervisor/scripts/build-bundle.js Outdated
Comment thread packages/xsnap-lockdown/scripts/build-bundle.js Outdated
t.is(passStyleOf(facet), 'remotable');
t.deepEqual(Object.getOwnPropertyNames(facet), []);
t.deepEqual(
Object.getOwnPropertyNames(facet).filter(name => !name.startsWith('__')),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Factor out reusable testing helper?

Comment thread packages/zoe/test/unitTests/test-zoe.js Outdated
await t.throwsAsync(() => E(zoe).getPublicFacet(), {
message:
/In "getPublicFacet" method of \(ZoeService\): arg 0: .*"\[undefined\]" - Must be a remotable/,
// Or pattern of golden error tolerant across versions of endo.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I know I wrote this horrible sentence, but how about

Suggested change
// Or pattern of golden error tolerant across versions of endo.
// Golden test uses RegExp "Or" pattern to tolerate earlier versions of endo

Likewise below

const seat = makeZCFSeat();
t.deepEqual(getStringMethodNames(seat), expectedStringMethods.sort());
t.deepEqual(
getStringMethodNames(seat).filter(name => !name.startsWith('__')),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you create reusable testing helper for these, see if you can use it here too.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It’s my preference to keep tests readable at the expense of repetition. In this case, I believe the code is offering an explanation.

/** @type {import('.').Zone['exoClass']} */
const exoClass = (...args) => prepareExoClass(baggage, ...args);
/** @type {import('.').Zone['exoClassKit']} */
// @ts-ignore This type check regressed inexplicably with the release

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see lint gave you the expected second order error. I have suppressed this either with another @ts-ignore or an eslint-disable-next-line . I forget which. In any case, I suspect plenty of these remain in the code.

Comment thread packages/time/package.json
@kriskowal kriskowal force-pushed the kriskowal-sync-endo-2023-11 branch 2 times, most recently from 763092b to 98e6c2b Compare December 11, 2023 23:14
@kriskowal kriskowal force-pushed the kriskowal-sync-endo-2023-11 branch from 98e6c2b to 454c83e Compare December 11, 2023 23:45
@kriskowal kriskowal force-pushed the kriskowal-sync-endo-2023-11 branch from 454c83e to 477003b Compare December 12, 2023 01:00
@kriskowal kriskowal force-pushed the kriskowal-sync-endo-2023-11 branch from 477003b to 8b1e82e Compare December 12, 2023 01:17
@kriskowal

Copy link
Copy Markdown
Member Author

Subsumed this into #8651 which goes on to sync up with published Endo versions.

@kriskowal kriskowal closed this Dec 12, 2023
@erights

erights commented Dec 12, 2023

Copy link
Copy Markdown
Member

I can also close #8642 as subsumed, yes?

@kriskowal kriskowal deleted the kriskowal-sync-endo-2023-11 branch November 13, 2024 20:31
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