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

chore: moddable 7abc931d 2021-03-19 XS: 608 (weak maps) #2689

Merged
merged 1 commit into from
Mar 19, 2021

Conversation

dckc
Copy link
Member

@dckc dckc commented Mar 19, 2021

again, semi-automated #2648

fixes #2670
I checked: Moddable-OpenSource/moddable#608 (comment)

@dckc dckc requested review from warner and kriskowal March 19, 2021 19:24
@dckc dckc marked this pull request as draft March 19, 2021 19:38
@dckc
Copy link
Member Author

dckc commented Mar 19, 2021

oops...

@dckc dckc marked this pull request as ready for review March 19, 2021 19:43
@dckc
Copy link
Member Author

dckc commented Mar 19, 2021

there.

Copy link
Member

@erights erights left a comment

Choose a reason for hiding this comment

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

What should one think about to review this?

In my ignorance, LGTM

@warner
Copy link
Member

warner commented Mar 19, 2021

I certainly don't know enough about their codebase to pass judgement.. I'm entirely at their mercy. I look for consistency in the changes that were made, and I treat it as an opportunity to see that some functional area was modified in some way. E.g. from the diffs I can tell that WeakMaps are doing something different (so yay maybe it fixes our bug), some sort of stack-size limit is being computed or maybe enforced (so yay progress towards some of our metering goals), and a number of places that were assuming a chunk size of 1 are now accepting a configurable chunk size, so some sort of memory allocation thing is becoming more generic. Not much for me to approve or disapprove, but it acts a bit like a ChangeLog of the upstream sources, which is nice to be aware of.

Copy link
Member

@warner warner left a comment

Choose a reason for hiding this comment

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

No problems jump out at me.

@dckc dckc enabled auto-merge (rebase) March 19, 2021 23:23
@dckc dckc merged commit ccf2db4 into master Mar 19, 2021
@dckc dckc deleted the moddable-xs-update branch March 19, 2021 23:37
@dckc dckc restored the moddable-xs-update branch April 3, 2021 17:12
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.

XS-specific test failure, WeakMap vs GC?
3 participants