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 CredentialsManagement public key options and other missing bits #1763

Merged
merged 12 commits into from
Oct 2, 2024

Conversation

autonome
Copy link
Collaborator

Date- and spec-wise this seems to all jive w/ the original feature, no baseline changes.

The only weird bit is that Caniuse seems incorrect here, unless they're only counting "support" as also supporting some of the optional parameters (unlikely, and would be strange): https://caniuse.com/credential-management. But that doesn't change this PR, just a side note.

@github-actions github-actions bot added the feature definition Creating or defining new features or groups of features. label Sep 10, 2024
@autonome autonome marked this pull request as draft September 10, 2024 16:01
@autonome
Copy link
Collaborator Author

In the spirit of "small patches for higher velocity", not going to blow this up into larger Credentials patch. Instead will do it in series of smaller chunks+refactors.

@autonome autonome marked this pull request as ready for review September 11, 2024 12:35
@ddbeck
Copy link
Collaborator

ddbeck commented Sep 20, 2024

Hmm, I find this to be a hard area to understand (we had to call in help from a Chrome engineer on #868). Do the extensions to create and get not belong with the webauthn feature? That's where most of the PublicKeyCredential stuff lives already. If not, why not?

@autonome autonome marked this pull request as draft September 24, 2024 07:45
@autonome
Copy link
Collaborator Author

Hrm, I did not search BCD keyspace first... yeah this set of features is sprinkled all over. Moved these to the right home at first glance, but need to look more, flipping back to draft.

@autonome
Copy link
Collaborator Author

autonome commented Sep 25, 2024

Ok, checked spec, and reviewed the various versions these additions shipped in - these do not justify a separate feature, and don't change baseline status of either webauthn or fedcm. Requesting review from @nsatragno as well, per the original feature addition.

Update: Looks like I don't have permissions to request review from individuals 🫤

@autonome autonome marked this pull request as ready for review September 25, 2024 06:12
Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

I have one minor suggestion. It'd be nice to get Nina's subject-matter expert review on it, but I have pretty high confidence in this one if she doesn't get to it by the end of the week.

features/fedcm.yml Outdated Show resolved Hide resolved
@ddbeck ddbeck merged commit dce9760 into web-platform-dx:main Oct 2, 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