rename to credential_configuration_id #162
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 4 commits
December 20, 2023 17:36
…authorization_details-authorization-request-parameter' of https://github.com/openid/OpenID4VCI into credential_configuration_id
Sakurann
requested review from
danielfett,
tlodderstedt,
pmhsfelix,
tplooker and
paulbastian
|
there is some diff not present in Paul's original PR, which i think is caused by paul's branch not being up to date with main. should be fixed in this PR when paul's branch I am pushing to is up to date with main. |
|
I think it is ok for this PR to be merged before pr #155 is merged, to prevent breaking changes post ID-1 |
danielfett
reviewed
Dec 21, 2023
danielfett
approved these changes
Dec 21, 2023
jogu
approved these changes
Dec 21, 2023
peppelinux
previously requested changes
Dec 21, 2023
awoie
reviewed
Dec 21, 2023
awoie
requested changes
Dec 21, 2023
paulbastian
approved these changes
Dec 21, 2023
…authorization_details-authorization-request-parameter' of https://github.com/openid/OpenID4VCI into credential_configuration_id
Co-authored-by: Giuseppe De Marco <[email protected]> Co-authored-by: Daniel Fett <[email protected]>
…authorization_details-authorization-request-parameter' into credential_configuration_id
…id/OpenID4VCI into credential_configuration_id
awoie
approved these changes
Dec 22, 2023
tlodderstedt
approved these changes
Dec 22, 2023
| @@ -14,7 +14,7 @@ | |||
| "locale": "fr-FR" | |||
| } | |||
| ], | |||
| "credentials_supported": { | |||
| "credential_configurations_supported": { | |||
There was a problem hiding this comment.
I think credential_configurations is enough (as this is a credential offer already).
c2bo
approved these changes
Dec 22, 2023
There was a problem hiding this comment.
I agree with @tlodderstedt that credential_configurations might be a better/easier name - apart from that it looks fine imho
|
good point - renamed to |
Co-authored-by: Oliver Terbu <[email protected]>
Sakurann
commented
Dec 22, 2023
Sakurann
dismissed
peppelinux’s stale review
some of the suggested changes applied. for the rest of the bikeshedding, please do in PR 155 as this PR is scoped to changing the parameter names only and not to have a perfect terminology description.
Sakurann
merged commit ab32991
into
102-reuse-credential-offer-identifier-from-metadata-in-authorization_details-authorization-request-parameter
Sakurann
added a commit
that referenced
this pull request
Dec 22, 2023
…etadata (#137) 5 approvals. no objections to merge during dec-21-2023 dcp wg call. open for more than a week. * change `authorization_details` to use `credential_description` pointing to the name of a `credentials_supported` object in the Credential Issuer's Metadata * Update openid-4-verifiable-credential-issuance-1_0.md Co-authored-by: Kristina <[email protected]> * Update openid-4-verifiable-credential-issuance-1_0.md Co-authored-by: Kristina <[email protected]> * rename `credential_description` to `credential` * Rename credential to credentials_supported_id agreed on in DCP call * Update openid-4-verifiable-credential-issuance-1_0.md Co-authored-by: Brian Campbell <[email protected]> * remove unnecessary parameters from the authoritazion details in Appendix E * fix json in example * rename to credential_configuration_id (#162) 6 approvals. general agreement to merge this during Dec-21-2023 WG call. some comments remain to be addressed in other PRs (mainly 155), as this PR is scoped to change the parameter name changes and not to have a perfect terminology. * change to credential_configuration_id * change to credential_configurations_offered * rename to credential_configurations_supported * Apply suggestions from code review Co-authored-by: Giuseppe De Marco <[email protected]> Co-authored-by: Daniel Fett <[email protected]> * rename to credential_configurations credential offer parameter * clarify there is no polymorphism Co-authored-by: Oliver Terbu <[email protected]> * editorial --------- Co-authored-by: Kristina <=> Co-authored-by: Giuseppe De Marco <[email protected]> Co-authored-by: Daniel Fett <[email protected]> Co-authored-by: Oliver Terbu <[email protected]> --------- Co-authored-by: Kristina <[email protected]> Co-authored-by: Brian Campbell <[email protected]> Co-authored-by: Giuseppe De Marco <[email protected]> Co-authored-by: Daniel Fett <[email protected]> Co-authored-by: Oliver Terbu <[email protected]>
|
this PR addresses renaming part of issue #91 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
applying three changes:
this PR needs to be merged before #137