Skip to content

feat(proto): Mark PathEvent as #[non_exhaustive]#648

Open
matheus23 wants to merge 2 commits into
mainfrom
matheus23/path-event-non-exhaustive
Open

feat(proto): Mark PathEvent as #[non_exhaustive]#648
matheus23 wants to merge 2 commits into
mainfrom
matheus23/path-event-non-exhaustive

Conversation

@matheus23
Copy link
Copy Markdown
Member

Description

This is for future-proofing the API.

Closes #642

Breaking Changes

  • enum PathEvent is now marked as #[non_exhaustive]

Notes & open questions

I'm ignoring the _ => case, as anything else would mean we would "do something" in case e.g. noq is bound against a newer noq-proto.
In #[cfg(test)] however, I panic, as in that case the version we depend on must be up to date.

Change checklist

  • Self-review.

@matheus23 matheus23 self-assigned this May 12, 2026
@github-actions
Copy link
Copy Markdown

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/noq/pr/648/docs/noq/

Last updated: 2026-05-12T14:25:21Z

@github-actions
Copy link
Copy Markdown

Performance Comparison Report

f82023fb1cda3d4fe1df9f581861420ec9be6a91 - artifacts

Raw Benchmarks (localhost)

Scenario noq upstream Delta CPU (avg/max)
large-single 5659.8 Mbps 7948.1 Mbps -28.8% 97.5% / 148.0%
medium-concurrent 5395.0 Mbps 8028.9 Mbps -32.8% 95.9% / 147.0%
medium-single 4384.6 Mbps 4580.7 Mbps -4.3% 92.0% / 101.0%
small-concurrent 3809.5 Mbps 5261.4 Mbps -27.6% 94.3% / 101.0%
small-single 3588.7 Mbps 4801.6 Mbps -25.3% 98.0% / 151.0%

Netsim Benchmarks (network simulation)

Condition noq upstream Delta
ideal 3063.9 Mbps N/A N/A
lan 782.4 Mbps N/A N/A
lossy 69.8 Mbps N/A N/A
wan 83.8 Mbps N/A N/A

Summary

noq is 25.4% slower on average

@n0bot n0bot Bot added this to iroh May 12, 2026
@github-project-automation github-project-automation Bot moved this to 🚑 Needs Triage in iroh May 12, 2026

/// Application events about paths
#[derive(Debug, Clone, PartialEq, Eq)]
#[non_exhaustive]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we need non-exhaustive on the event fields as well. I think e.g. iroh would benefit from adding the remote address to Established and that would be nice to be able to do without breaking the API

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🚑 Needs Triage

Development

Successfully merging this pull request may close these issues.

PathEvent should be non-exhaustive

2 participants