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

Add more fetch keys #1982

Merged
merged 8 commits into from
Oct 18, 2024
Merged

Add more fetch keys #1982

merged 8 commits into from
Oct 18, 2024

Conversation

autonome
Copy link
Collaborator

No description provided.

@github-actions github-actions bot added the feature definition Creating or defining new features or groups of features. label Oct 16, 2024
@autonome autonome marked this pull request as draft October 16, 2024 08:51
@autonome autonome marked this pull request as ready for review October 17, 2024 09:53
Copy link
Collaborator

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

My review approach here is a comparison against the burndown of non-deprecated and standardized BCD keys for the interfaces mentioned and so I found more keys to consider here. But it could also be that some keys want to live in a separate feature or already do, so take the suggestions as inspiration rather than blockers.

features/fetch.yml Show resolved Hide resolved
features/fetch.yml Show resolved Hide resolved
features/fetch.yml Show resolved Hide resolved
features/fetch.yml Show resolved Hide resolved
features/xhr.yml Outdated Show resolved Hide resolved
@autonome
Copy link
Collaborator Author

My review approach here is a comparison against the burndown of non-deprecated and standardized BCD keys for the interfaces mentioned and so I found more keys to consider here. But it could also be that some keys want to live in a separate feature or already do, so take the suggestions as inspiration rather than blockers.

this is great, thanks! i found the additional keys in this PR... somewhere (?), so added them. definitely will add these others and see what it looks like.

@autonome
Copy link
Collaborator Author

Aha i see your other patch now! I'll limit this to fetch, so we don't conflict, and you can do all the xhr additions in #1963.

@autonome
Copy link
Collaborator Author

added all, looks good, thanks @Elchi3!

@autonome autonome requested a review from Elchi3 October 18, 2024 04:41
@autonome autonome marked this pull request as draft October 18, 2024 04:43
@autonome
Copy link
Collaborator Author

Did a pass to see if any of these keys should be separate features.

Caniuse doesn't break any sub features out, just has Fetch and shows the baseline high widget.

There's a multi-year trickle of additions for blob support, readable stream integration and misc bits and bobs, some of which are baseline false.

There's nothing that seems to stand out from the implementation timeline, nor from the docs or dev comments/writing based on the searches I did.

There aren't any keys that match the other sub-features (metadata and priority).

So leaving it all in here for now.

@autonome autonome marked this pull request as ready for review October 18, 2024 07:59
Copy link
Collaborator

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

Nice, this looks good to me. Great to see all ducks in a row for fetch. I think we can always move stuff out into a separate feature later if there is appetite.

@Elchi3 Elchi3 merged commit 1494126 into web-platform-dx:main Oct 18, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature definition Creating or defining new features or groups of features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants