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

Unify Error Handling #64

Merged
merged 5 commits into from
Jun 30, 2024
Merged

Unify Error Handling #64

merged 5 commits into from
Jun 30, 2024

Conversation

Wind4Greg
Copy link
Collaborator

@Wind4Greg Wind4Greg commented Jun 24, 2024

This PR addresses issue #63. It unifies error codes, error handling language, and adds error codes where needed.


Preview | Diff

index.html Outdated Show resolved Hide resolved
Copy link
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

Editorial, punctuation and phrasing, for consistency and clarity

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated
</li>
<li>
If |proofConfig|.|created| is set and if the value is not a
valid [[XMLSCHEMA11-2]] datetime, an `INVALID_PROOF_DATETIME` error MUST be
raised.
valid [[XMLSCHEMA11-2]] datetime, an an error MUST be raised and SHOULD convey
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
valid [[XMLSCHEMA11-2]] datetime, an an error MUST be raised and SHOULD convey
valid [[XMLSCHEMA11-2]] datetime, an error MUST be raised and SHOULD convey

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I got some weird GitHub errors when trying to batch in all the changes. So had to do it in two batches.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Wind4Greg and others added 2 commits June 25, 2024 08:22
…r consistency and clarity.

Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Dave Longley <[email protected]>
…r consistency and clarity.

Co-authored-by: Ted Thibodeau Jr <[email protected]>
@Wind4Greg Wind4Greg requested a review from TallTed June 25, 2024 15:27
@msporny msporny added normative This item is a normative change. CR1 labels Jun 30, 2024
@msporny
Copy link
Member

msporny commented Jun 30, 2024

Normative, multiple reviews, changes requested and made, no objections, merging.

@msporny msporny merged commit 5daa501 into w3c:main Jun 30, 2024
1 check passed
TallTed added a commit that referenced this pull request Jul 1, 2024
- delete now-errant line

- fix "ensure" phrasing — note: this sentence may need further work, as it's not clear whether confirming `an array of five elements` is sufficient to populate `the five elements` --
  ```
  Initialize |components| to an array that is the result of CBOR-decoding the
  bytes that follow the three-byte ECDSA-SD base proof header. Ensure the result
  is an array of five elements.
              </li>
              <li>
  Return an object with properties set to the five elements, using the names
  `baseSignature`, `publicKey`, `hmacKey`, `signatures`, and `mandatoryPointers`,
  respectively.
  ```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR1 normative This item is a normative change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants