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

Error inspection #4660

Merged
merged 79 commits into from
Oct 27, 2023
Merged

Error inspection #4660

merged 79 commits into from
Oct 27, 2023

Conversation

samcowger
Copy link
Contributor

@samcowger samcowger commented Aug 26, 2023

Description

This PR contains results of Galois's inspection of the use of error in ouroboros-network packages. It consists of an enumeration of error uses and my notes on each. I was able to document, modify, or remove some errors immediately; others would benefit from more information or clarification from someone more familiar with this codebase.

Viewers/reviewers may examine the diff first and my notes (currently at 3836/3836.md) second, or vice versa. Everything in the diff should have a corresponding entry in the notes, but not every entry in the notes has a corresponding diff.

This is marked as a draft because there are a couple of changes that may warrant additional unit tests, and because I recognize my notes will need to be relocated or removed before merging, but I wanted to get eyes on this work sooner rather than later. I'll be going through this to annotate (in Github) items that I feel especially warrant another set of eyes.

Fixes #3836.

cc @coot @mtullsen

Checklist

  • Branch
    • Updated changelog files.
    • Commit sequence broadly makes sense
    • Commits have useful messages
    • The documentation has been properly updated
    • New tests are added if needed and existing tests are updated
    • [N/A] If serialization changes, user-facing consequences (e.g. replay from genesis) are confirmed to be intentional.
  • Pull Request
    • Self-reviewed the diff
    • Useful pull request description at least containing the following information:
      • What does this PR change?
      • Why these changes were needed?
      • How does this affect downstream repositories and/or end-users?
      • Which ticket does this PR close (if any)? If it does, is it linked?
    • Reviewer requested

@samcowger samcowger marked this pull request as ready for review August 29, 2023 16:26
3836/3836.md Outdated Show resolved Hide resolved
3836/3836.md Outdated Show resolved Hide resolved
3836/3836.md Outdated Show resolved Hide resolved
3836/3836.md Outdated Show resolved Hide resolved
3836/3836.md Outdated Show resolved Hide resolved
3836/3836.md Outdated Show resolved Hide resolved
3836/3836.md Outdated Show resolved Hide resolved
3836/3836.md Outdated Show resolved Hide resolved
3836/3836.md Outdated Show resolved Hide resolved
3836/3836.md Outdated Show resolved Hide resolved
Copy link
Contributor

@coot coot left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for a thorough review :).

samcowger added a commit to samcowger/ouroboros-network that referenced this pull request Oct 18, 2023
See IntersectMBO#4660

Co-authored-by: Marcin Szamotulski <coot@coot.me>
samcowger added a commit to samcowger/ouroboros-network that referenced this pull request Oct 18, 2023
See IntersectMBO#4660

Co-authored-by: Marcin Szamotulski <coot@coot.me>
samcowger added a commit to samcowger/ouroboros-network that referenced this pull request Oct 23, 2023
See IntersectMBO#4660

Co-authored-by: Marcin Szamotulski <coot@coot.me>
@samcowger samcowger mentioned this pull request Oct 25, 2023
samcowger added a commit to samcowger/ouroboros-network that referenced this pull request Oct 26, 2023
samcowger and others added 23 commits October 27, 2023 12:27
…c resolution

Before, we would find the domain that corresponded to an async action only after
the action completed/errored, by looking it up in the original list of
actions. Now, we carry the domain through the process of awaiting the async's
completion/erroring.
Co-authored-by: Marcin Szamotulski <coot@coot.me>
See IntersectMBO#4660

Co-authored-by: Marcin Szamotulski <coot@coot.me>
The document this deletes has been migrated to issue IntersectMBO#3836.
@coot
Copy link
Contributor

coot commented Oct 27, 2023

@samcowger I rebased your PR.

@coot coot enabled auto-merge October 27, 2023 10:32
@coot coot added this pull request to the merge queue Oct 27, 2023
Merged via the queue into IntersectMBO:master with commit 7631338 Oct 27, 2023
10 of 12 checks passed
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.

Inspect use of error
3 participants