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

Spec deprecatedReplaceInURN #117

Merged
merged 6 commits into from
Sep 22, 2023
Merged

Spec deprecatedReplaceInURN #117

merged 6 commits into from
Sep 22, 2023

Conversation

gtanzer
Copy link
Collaborator

@gtanzer gtanzer commented Sep 6, 2023

spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated
[=node navigable=]'s [=navigable/traversable navigable=]'s
[=traversable navigable/fenced frame config mapping=].

1. Let |config| be the result of [=fenced frame config mapping/finding a config=] in |mapping|
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems wrong to do this synchronously, and seems to not match what the implementation does, since the implementation does this "in parallel" and returns a promise.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I should make this a promise...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Turned this into a promise, modeling after some parts of the Protected Audience spec. Lmk if it I didn't do it right.

spec.bs Outdated Show resolved Hide resolved
@@ -1222,6 +1222,78 @@ Each {{FencedFrameConfig}} has:
1. Initialize |value|'s [=fencedframeconfig/contentHeight=] to |serialized|.\[[ContentHeight]].
</div>

To help with ease of adoption,
[until 2026](https://github.com/WICG/turtledove/issues/286#issuecomment-1682842636) we will support
Copy link
Collaborator

Choose a reason for hiding this comment

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

This makes it sound like we're actually indeed removing the API in 2026. Are we? Or are we just "deprecating" it softly then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It has been deprecated since it was added, hence the name. It should actually be removed in several years.

spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
@domfarolino domfarolino merged commit 3b93e20 into master Sep 22, 2023
2 checks passed
@domfarolino domfarolino deleted the deprecated-replace-in-urn branch September 22, 2023 14:39
github-actions bot added a commit that referenced this pull request Sep 22, 2023
SHA: 3b93e20
Reason: push, by domfarolino

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

2 participants