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.

Sign up for GitHub

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

Jump to bottom

Refactor Streams and Connections to include Stat properties #1849

Closed
maschad opened this issue May 18, 2023 · 0 comments · Fixed by #1856
Closed

Refactor Streams and Connections to include Stat properties #1849

maschad opened this issue May 18, 2023 · 0 comments · Fixed by #1856
Labels
good first issue Good issue for new contributors help wanted Seeking public contribution on this issue

Comments

@maschad
Copy link
Member

maschad commented May 18, 2023 •
edited
Loading 

          I think maybe it'd be better if we broke all the properties of `.stat` out from the streams and connections and just had them on the stream/connection.  It's not clear (to me) what belongs in the `.stat` property and what belongs on the stream/connection.

It'd make it simpler to extend the interfaces and we also wouldn't need to have deeply nested property accessors then either.

@wemeetagain what do you think?

Originally posted by @achingbrain in libp2p/js-libp2p-interfaces#265 (comment)

Refactor the Stream and Connection stat properties should be at the top level for ease of extendability. This would enforce that negotiated streams have a protocol for instance and avoid these unnecessary types of non-nullish assertions
@maschad maschad mentioned this issue May 18, 2023
Closed
@maschad maschad added good first issue Good issue for new contributors help wanted Seeking public contribution on this issue labels Jun 7, 2023
@achingbrain achingbrain transferred this issue from libp2p/js-libp2p-interfaces Jun 26, 2023
achingbrain added a commit that referenced this issue Jun 26, 2023
@achingbrain
feat!: merge stat properties into stream/connection objects
dffd58d 
Moves `direction`, `timeline`, `status`, etc from `.stat` objects
on `Stream` and `Connection` objects onto the root object so we
don't have awkward constructs like `conn.stat.status`, instead the
simpler `conn.status`.

Fixes #1849

BREAKING CHANGE: `stream.stat.*` and `conn.stat.*` properties are now accessed via `stream.*` and `conn.*`
@achingbrain achingbrain mentioned this issue Jun 26, 2023
Merged
achingbrain added a commit that referenced this issue Jun 27, 2023
@achingbrain
feat!: merge stat properties into stream/connection objects
4a1f87a 
Moves `direction`, `timeline`, `status`, etc from `.stat` objects
on `Stream` and `Connection` objects onto the root object so we
don't have awkward constructs like `conn.stat.status`, instead the
simpler `conn.status`.

Fixes #1849

BREAKING CHANGE: `stream.stat.*` and `conn.stat.*` properties are now accessed via `stream.*` and `conn.*`
@github-project-automation github-project-automation bot moved this from 🛠️ Todo to 🎉Done in js-libp2p Jul 20, 2023
achingbrain added a commit that referenced this issue Jul 20, 2023
@achingbrain
feat!: merge stat properties into stream/connection objects (#1856)
e9cafd3 
Moves `direction`, `timeline`, `status`, etc from `.stat` objects on `Stream` and `Connection` objects onto the root object so we don't have awkward constructs like `conn.stat.status`, instead the simpler `conn.status`.

Fixes #1849

BREAKING CHANGE: `stream.stat.*` and `conn.stat.*` properties are now accessed via `stream.*` and `conn.*`
achingbrain added a commit that referenced this issue Jul 20, 2023
@achingbrain
feat!: merge stat properties into stream/connection objects
3cafbf4 
Moves `direction`, `timeline`, `status`, etc from `.stat` objects
on `Stream` and `Connection` objects onto the root object so we
don't have awkward constructs like `conn.stat.status`, instead the
simpler `conn.status`.

Fixes #1849

BREAKING CHANGE: `stream.stat.*` and `conn.stat.*` properties are now accessed via `stream.*` and `conn.*`
This was referenced Jan 18, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good issue for new contributors help wanted Seeking public contribution on this issue
Projects
js-libp2p
Archived in project
Milestone
No milestone
1 participant
@maschad