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

Start with trying to spec foreign fetch. #749

Merged
merged 1 commit into from Sep 18, 2015

Conversation

mkruisselbrink
Copy link
Collaborator

My attempt to spec the API described in issue #684.

I don't think this is anywhere near ready to be merged yet (in only because as far as I know current master is still V1, and this definitely is a V2 feature), but a pull request seemed like the easiest way to have a place to discuss details of foreign fetch.

Also haven't updated the .ts file with these changes, but then that file seems to be rather out of date anyway. So not sure if that file is still being used?

jungkees added a commit that referenced this pull request Sep 18, 2015
Thanks a lot @mkruisselbrink. I made a branch for V1 today, so we can work on nightly branch for it now. I think it's fine to start the work with this PR. I'll try to fix details, if needed. Re the .ts file, we decided to drop it as now we have full spec already.
@jungkees jungkees merged commit 28e694d into w3c:master Sep 18, 2015
@annevk
Copy link
Member

annevk commented Sep 18, 2015

@jungkees did you read OP? "I don't think this is anywhere near ready to be merged yet"...

@jungkees
Copy link
Collaborator

@mkruisselbrink was waiting for V1 branching out. I reviewed and merged this PR to the nightly version. I think it's okay. A few minor fixes seem to be needed which I'll take care.

@jungkees
Copy link
Collaborator

@mkruisselbrink thanks for the excellent text. I made minor changes (ca15dee) especially in InstallEvent and its event.registerForeignFetchScopes(subScopes) steps. Note that I changed the parameter type of the method to sequence<USVString> in order to maintain consistency with other methods (such as cache.addAll(sequence<RequestInfo> requests)) in the spec.

@wanderview
Copy link
Member

It still seems some time to review and discuss before merging to spec would be nice.

@jungkees
Copy link
Collaborator

Agreed and reverted the merge manually (3cc01ee) as I had already put some changes on it.

My apologies @mkruisselbrink. Could you make a PR again to the nightly version (the same file you made one)? Let's discuss and work on it. Thanks!

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.

None yet

4 participants