Skip to content

Misc Editorial #1456

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
Merged
ljharb merged 3 commits into from
Apr 12, 2019
Merged

Misc Editorial #1456

ljharb merged 3 commits into from
Apr 12, 2019

Conversation

@jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented Feb 27, 2019

4 commits re some of today's merges: #1311, #1250, #1309.

@ljharb ljharb requested review from a team, bterlson and zenparsing February 27, 2019 04:47
zenparsing
zenparsing approved these changes Feb 27, 2019
View reviewed changes
Copy link
Member

@zenparsing zenparsing left a comment

Choose a reason for hiding this comment

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

Thank you!

@ljharb ljharb self-assigned this Feb 27, 2019
@ljharb ljharb removed the request for review from bterlson February 28, 2019 19:52
@zenparsing
Copy link
Member

zenparsing commented Mar 20, 2019

@jmdyck It seems that #1486 fixes the module issues. If you agree, would you be opposed to removing those changes?

@jmdyck
Copy link
Collaborator Author

jmdyck commented Mar 21, 2019

#1486 fixes the issue I raised in #1455, but not the issue that the 4th commit of this PR addresses, namely that in For each steps, we generally give the most precise 'type' for the 'loop variable', but CMR's Instantiate() and Evaluate() algorithms don't.

In Evaluate(), #1486 adds an Assert of the more precise type as the first step of the body of the For each, but that's not something the spec does.

As for Instantiate(), #1486 doesn't do anything.

@jmdyck
Copy link
Collaborator Author

jmdyck commented Apr 11, 2019

(force-pushed revised commits to resolve merge conflicts)

jmdyck added 3 commits April 12, 2019 14:52
@jmdyck @ljharb
Editorial: Insert spaces for consistency; delete extra spaces (tc39#1456
0f23d3a 
)

(mostly in clause-headers)
@jmdyck @ljharb
Editorial: In 3 calls to PromiseResolve, fix the 2nd arg (tc39#1456)
306cc93 
It's an ECMAScript language value, not a List.
@jmdyck @ljharb
Editorial: Change "module" to "Cyclic Module Record" (tc39#1456)
e7b55c3 
... in "For each module _m_ in _stack_",
in the Instantiate() and Evaluate() methods for Cyclic Module Record.

_stack_ is built by InnerModuleInstantiation & InnerModuleEvaluation,
which only append Cyclic Module Records to it.
@ljharb ljharb merged commit e7b55c3 into tc39:master Apr 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ljharb ljharb ljharb approved these changes

+1 more reviewer

@zenparsing zenparsing zenparsing approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ljharb ljharb

Labels

editorial change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jmdyck @zenparsing @ljharb