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

Feature/operations handling, Business Rules #487

Merged
merged 55 commits into from Feb 8, 2023

Conversation

mprorock
Copy link
Collaborator

Corrects some formatting on HTML.
Reorders sections for better readability.
Adds section around business rules handling for identifiers and common operations performed using external systems with data receiving or analyzing data presented using this spec

@mprorock
Copy link
Collaborator Author

mprorock commented Jan 18, 2023

Full understanding that there is a fair amount of re-ordering, etc with this PR, and that it will take time to review. Given how out of order several sections were, e.g. vocab pointers buried below other sections in the spec, i would request that the spec is read top to bottom to ensure correct ordering of sections, per specification norms, etc.

new additions to the spec are confined to the section starting at line 358 "Rules for Processing Data"

docs/spec/index.html Outdated Show resolved Hide resolved
docs/spec/index.html Outdated Show resolved Hide resolved
docs/spec/index.html Outdated Show resolved Hide resolved
docs/spec/index.html Outdated Show resolved Hide resolved
docs/spec/index.html Outdated Show resolved Hide resolved
`TraceablePresentation.workflow.instance`, assuming that the
implementation's underlying data store persists presentation metadata
upon receiving that data. Rules for ensuring the retrieval of the most
"correct" credentails associated for that workflow instance are
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is the primary challenge for us to define... assuming a stream of presentations where credentials may be "replayed" or "replaced" or "updated"... what is the current state of the instance, and which credentials are most authoritative for satisfying the requirements of the definition.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

suggest we build on this, after it is merged

docs/spec/index.html Outdated Show resolved Hide resolved
docs/spec/index.html Outdated Show resolved Hide resolved
docs/spec/index.html Outdated Show resolved Hide resolved
Copy link
Collaborator

@OR13 OR13 left a comment

Choose a reason for hiding this comment

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

Overall looks good, let me know when its out of draft, and I will review again.

Copy link
Collaborator

@nissimsan nissimsan left a comment

Choose a reason for hiding this comment

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

Nice, Mike! Thank you.

docs/spec/index.html Outdated Show resolved Hide resolved
@TallTed
Copy link
Contributor

TallTed commented Jan 20, 2023

For future, separating radical re-ordering from pretty much any other change(s) makes reviewing much easier. This melange is gonna take a while to digest/understand, even if I review commit-by-commit...

@mkhraisha
Copy link
Collaborator

mkhraisha commented Jan 20, 2023

can you run npm run format:docs? it'll make reviewing much easier, as I assume it won't override the current lint.

Copy link
Collaborator

@OR13 OR13 left a comment

Choose a reason for hiding this comment

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

Only blocking feedback is related to "conversion to github issues" or "creating an issue"

@TallTed
Copy link
Contributor

TallTed commented Jan 31, 2023

174 comments. 45 commits. and counting. Most if not all without specific issues behind them.

This PR has gone past my ability to usefully review, largely because GitHub doesn't know how to highlight text changes of this variety or magnitude, and, so far as I can tell, there's no PR Preview.

I can't think of how to best proceed at this point. Maybe someone else has a good idea.

docs/spec/index.html Outdated Show resolved Hide resolved
docs/spec/index.html Outdated Show resolved Hide resolved
docs/spec/index.html Outdated Show resolved Hide resolved
docs/spec/index.html Outdated Show resolved Hide resolved
Co-authored-by: Orie Steele <orie@or13.io>
docs/spec/index.html Outdated Show resolved Hide resolved
brownoxford and others added 2 commits February 2, 2023 11:50
Co-authored-by: Orie Steele <orie@or13.io>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
docs/spec/index.html Outdated Show resolved Hide resolved
brownoxford and others added 4 commits February 2, 2023 12:02
Co-authored-by: Orie Steele <orie@or13.io>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Chris Abernethy <brownoxford@gmail.com>
@mprorock
Copy link
Collaborator Author

mprorock commented Feb 2, 2023

Only blocking feedback is related to "conversion to github issues" or "creating an issue"

I believe we now have issues open on everything

@mprorock mprorock requested a review from OR13 February 2, 2023 18:07
Copy link
Collaborator

@OR13 OR13 left a comment

Choose a reason for hiding this comment

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

A few additional issues need to be linked in respec, see:

https://respec.org/docs/#data-number

@OR13
Copy link
Collaborator

OR13 commented Feb 2, 2023

@mprorock sorry, I should have been more clear, opening issues is good, but for "new things" they also need to be linked in the spec, I have left comments on those, and left the rest of your resolutions resolved.

@mprorock
Copy link
Collaborator Author

mprorock commented Feb 2, 2023

@mprorock sorry, I should have been more clear, opening issues is good, but for "new things" they also need to be linked in the spec, I have left comments on those, and left the rest of your resolutions resolved.

last issue that did not have a data-number has been added in 3ae7a99

@OR13 OR13 self-requested a review February 2, 2023 18:26
@OR13
Copy link
Collaborator

OR13 commented Feb 8, 2023

I am merging this, its massive, and we need to start addressing follow up items.

@OR13 OR13 merged commit 696ae09 into w3c-ccg:main Feb 8, 2023
@brownoxford brownoxford deleted the feature/operations-handling branch February 8, 2023 16:09
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

6 participants