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

Update twotypes-env.txt #347

Closed
wants to merge 3 commits into from
Closed

Update twotypes-env.txt #347

wants to merge 3 commits into from

Conversation

nedmsmith
Copy link
Collaborator

Added an optional attesting environment that supplies evidence to a lead attesting env.

Copy link
Collaborator

@thomas-fossati thomas-fossati left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@nedmsmith
Copy link
Collaborator Author

Circlecl fails to build but it builds successfully on my local machine. Someone familiar with circlecl should take a look.

@thomas-fossati
Copy link
Collaborator

Circlecl fails to build but it builds successfully on my local machine. Someone familiar with circlecl should take a look.

See martinthomson/i-d-template#293

TL;DR drop CircleCI and move to GitHub Actions :-)

@mcr
Copy link
Collaborator

mcr commented Jul 6, 2021

Circlecl fails to build but it builds successfully on my local machine. Someone familiar with circlecl should take a look.

See martinthomson/i-d-template#293

TL;DR drop CircleCI and move to GitHub Actions :-)

It's not that simple, as I'm finding with another repo.

@mcr
Copy link
Collaborator

mcr commented Jul 6, 2021

But, about the proposed change to the diagram.
I don't think that this helps with understanding at this point in the document.

@thomas-fossati
Copy link
Collaborator

But, about the proposed change to the diagram.
I don't think that this helps with understanding at this point in the document.

It's not made explicit In the PR, but Ned's patch addresses issue #344. Maybe let's discuss there, in particular if you don't think it's an issue worth fixing.

@henkbirkholz
Copy link
Member

I'll take on the editorial task to align the diagram with the intended proposal for change here, if this does not work out-of-the-box in the current tool chain. Are there any objections with my assessment that this is again just an editorial fix: in this case we are aligning a diagram to reflect text better?

If there are no counter indications, I'll fix this next week¹

[¹] soon (tm)

@henkbirkholz
Copy link
Member

This kinda got old and we'll discuss it at the next editor's meeting.

@henkbirkholz
Copy link
Member

fixes #344

@nedmsmith
Copy link
Collaborator Author

If tweaking this diagram, the "Collect Claims" arrow could be misinterpreted as Target sending claims to AE. If arrow text was "Collected Claims" then its less likely to be misinterpreted.

@mcr mcr added the wontfix Should respond via email but does not warrant doc changes label Jan 28, 2022
@henkbirkholz henkbirkholz removed the wontfix Should respond via email but does not warrant doc changes label Feb 1, 2022
@mcr
Copy link
Collaborator

mcr commented Feb 3, 2022

I have added this diagram as a second digram, and moved the diagram to the end of section 3.

## Environment Interactions

As explained in the sections below, the Attesting Environment may be arbitrarily complex.
It may be distributed in various ways, or may depend upon other environments that collect claims on the environment itself as part of the process.
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does "distributed" mean here? Distributing code for an attester? Maybe pick another term?

| .-------------. .-------------. |
| | Attesting | | Other | |
| | Environment |<------------| Attesting | |
| | | Evidence | Environment | |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this Evidence different from the Evidence in the vertical arrow?
Assuming so, then Evidence' or Evidence 2 would be more clear, but either way the text discussing the diagram should explain what happens with this Evidence, since right now I don't understand this diagram.

@mcr mcr closed this Feb 8, 2022
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

5 participants