Skip to content

Conversation

@littledan
Copy link
Member

The terminology between WebAssembly and ES modules differs when it comes to
instantiation, which causes some confusion for the integration of the two
systems. Ideally, we wouldn't use the same term to mean two different things
between the specs.

After talking about it with multiple people, it seems like calling it "the
linking phase" might be a better name for this phase anyway. Developers
are often confused about what actually happens during instantiation and
why it's separated from evaluation. I believe "linking" more clearly
communicates the justification for this being its own phase.

@littledan
Copy link
Member Author

This patch is a rebase of #1312

@ljharb
Copy link
Member

ljharb commented Mar 3, 2019

I’ll update the original PR; please note that by opening a second PR (instead of just posting a link to the commits) i now have to keep both in sync to avoid stale refs on the repo.

@littledan
Copy link
Member Author

The original PR author had asked me to push forward the proposal, but I don't have write access to her repo. I don't want to make extra work for you. What do you recommend I do in this sort of case? (Also, I am curious, what do you mean by stale refs?)

@ljharb
Copy link
Member

ljharb commented Mar 4, 2019

Ideally, the PR author would give you write access to the fork - second to that, update a branch on your fork, and post a link to it on the PR (and then an editor can update the author's fork to match your branch).

(For every PR, github makes an origin/pull/123 ref (that's not fetched by default, but is still there). If this one was merged without updating #1312, then origin/pr/1312 would end up orphaned, never merged into master. This is a big part of the reason why github's "squashmerge" and "rebasemerge" features are undesirable)

@annevk
Copy link
Member

annevk commented Mar 4, 2019

@littledan I suspect you do have write access. Even I see

Add more commits by pushing to the rename-instantiate branch on linclark/ecma262.

presumably because you added me to the tc39 organization recently.

@ljharb I don't understand, it's not like you keep all old branches around?

@littledan
Copy link
Member Author

@annevk Oh, you're right, I do have write access. Closing this branch then, since I rebased the original.

@ljharb Do you have a workflow in mind for how people should make use of the metadata that the editors strive to maintain? It may be useful to document this in the how-we-work repository. The decision to move away from "squashmerge" and "rebasemerge" have been controversial in the committee, and it's been unclear what the motivation is; it's nice to hear a bit about the reasoning here (though, like @annevk , I feel like I'm still missing something). Explaining the motivation to the committee could help us to make less work for you; I had no idea that it was considered undesirable to have a PR that didn't eventually merge.

@littledan littledan closed this Mar 4, 2019
@domenic
Copy link
Member

domenic commented Mar 4, 2019

+1 to @littledan's concerns; I feel that there are a lot of contribution-hostile Git rules in place for this repo that are not documented.

@ljharb
Copy link
Member

ljharb commented Mar 4, 2019

@annevk not old branches, but all PR refs live forever.

@littledan this needs to stay open so i can keep them in sync.

@domenic i think “hostile” is a bit hyperbolic; but this PR isn’t the place to discuss that.

I’m certainly happy to document things better.

@ljharb ljharb reopened this Mar 4, 2019
@littledan
Copy link
Member Author

What's needed to land this PR?

@ljharb ljharb closed this Sep 2, 2019
@ljharb ljharb force-pushed the rename-instantiate branch from 2cbf848 to 05c7620 Compare September 2, 2019 14:33
@ljharb
Copy link
Member

ljharb commented Sep 2, 2019

Thanks for the reminder; I've force pushed #1312's commit onto this PR branch, so it's auto-closed and the PR ref has been updated to match the other PR.

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.

5 participants