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

Specify roles/components in endpoint steps for Supply Chain Import #5

Merged
merged 3 commits into from
Jan 25, 2022

Conversation

bumblefudge
Copy link
Collaborator

@bumblefudge bumblefudge commented Jan 11, 2022


Preview | Diff

Eric Schuh and others added 2 commits December 21, 2021 13:06
…t is currently defined, out of scope for the API, or does not exist but is in scope
@eric-schuh
Copy link
Collaborator

There are some formatting issues but overall looks fine for our transition stage. Think we want to get to diagrams like the one attache
UC1 - Get Digital Permanent Resident Card
d

Copy link
Contributor

@jandrieu jandrieu left a comment

Choose a reason for hiding this comment

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

I didn't mark up all of the out-of-scope statements. I recommend they all be removed.

I also found the textual sequence diagram incredibly hard to read. I recommend we use a visual diagram.

index.html Outdated Show resolved Hide resolved
<li>Holder (SMG) --> Holder App --> Verifier App: Notify ( GET /presentations/available ) </li>
<li>Holder App <-- Verifier App: return Domain & Challenge </li>
<li> {Holder App --> Holder Storage: get VCs ( out of scope )}</li>
<li> {Holder App --> Holder Storage and/or KMS: get keys ( out of scope )}</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand this one. Is it to the storage or to the KMS? And why are we getting keys? IMO, private keys should never be transmitted.

Suggested change
<li> {Holder App --> Holder Storage and/or KMS: get keys ( out of scope )}</li>

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 was in the diagram from your meeting with them, but omitted in the corresponding use-case. i wasn't at the meeting so I have no context

Co-authored-by: Joe Andrieu <joe@andrieu.net>
@eric-schuh eric-schuh merged commit 417e20e into main Jan 25, 2022
@eric-schuh eric-schuh deleted the addEndpoints branch January 25, 2022 20:37
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

3 participants