Skip to content
This repository has been archived by the owner on Jun 26, 2023. It is now read-only.

feat!: add upgrader options #290

Merged
merged 2 commits into from
Oct 6, 2022
Merged

feat!: add upgrader options #290

merged 2 commits into from
Oct 6, 2022

Conversation

ckousik
Copy link
Contributor

@ckousik ckousik commented Sep 27, 2022

Allow upgrader to accept a StreamMuxerFactory and skip encryption for transports that inherently support encryption and/or multiplexing (WebRTC, WebTransport, etc.)

@ckousik
Copy link
Contributor Author

ckousik commented Sep 27, 2022

@mxinden @MarcoPolo

@mxinden mxinden mentioned this pull request Oct 3, 2022
Merged
19 tasks
@BigLep BigLep requested a review from achingbrain October 3, 2022 14:27
This was referenced Oct 3, 2022
Closed
Closed
@BigLep
Copy link

BigLep commented Oct 4, 2022

Per #292 (comment)

Fixes: libp2p/js-libp2p#1382 and #287

Enables WebTransport: https://github.com/libp2p/js-libp2p-webtransport/

@achingbrain achingbrain changed the title Add Upgrader Options feat: add Upgrader Options Oct 4, 2022
@achingbrain achingbrain changed the title feat: add Upgrader Options feat: add upgrader options Oct 4, 2022
@ckousik ckousik mentioned this pull request Oct 5, 2022
Closed
MarcoPolo
MarcoPolo reviewed Oct 5, 2022
View reviewed changes
packages/interface-stream-muxer/package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "@libp2p/interface-stream-muxer",
"version": "2.0.2",
"version": "2.0.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we avoid this too for the same reason as: #290 (comment) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, sorry. Will fix

@MarcoPolo MarcoPolo requested a review from achingbrain October 5, 2022 15:55
@ckousik
Copy link
Contributor Author

ckousik commented Oct 5, 2022

@MarcoPolo wouldn't we have to update the patch version since we have added features?

@MarcoPolo
Copy link
Contributor

@achingbrain made it sound like the version numbers are handled by CI during release. I don't know much more.

@mpetrunic
Copy link
Member

@MarcoPolo wouldn't we have to update the patch version since we have added features?

If you added a feature, a minor version should be updated. But in this case, since the title contains "feat:", CI will automatically update versions where needed.

achingbrain
achingbrain suggested changes Oct 5, 2022
View reviewed changes
Copy link
Member

@achingbrain achingbrain left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, just needs the extra boolean option adding.

IMPORTANT: this is a breaking change. Before merging the following needs to happen:

  1. The title of this PR needs to change from feat: add upgrader options to feat!: add upgrader options
  2. The commit message needs to have the following added to it when this PR is squashed and merged (the format is important - "BREAKING CHANGE: " followed by a one-line sentence - this will be added to the release notes and changelog):
BREAKING CHANGE: the return type of StreamMuxer.newStream can now return a promise

Either one of the above two changes will make semantic-release rev the version numbers correctly when CI runs on master after merge. Both changes will make doubly sure it doesn't miss it.

packages/interface-stream-muxer/src/index.ts
@@ -18,7 +18,7 @@ export interface StreamMuxer extends Duplex<Uint8Array> {
* Initiate a new stream with the given name. If no name is
* provided, the id of the stream will be used.
*/
newStream: (name?: string) => Stream
newStream: (name?: string) => Stream | Promise<Stream>
Copy link
Member

@achingbrain achingbrain Oct 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Based on our learnings from #295 this is a breaking change.

Making this change locally with libp2p master indeed fails to compile.

packages/interface-transport/src/index.ts Outdated Show resolved Hide resolved
@MarcoPolo MarcoPolo changed the title feat: add upgrader options feat!: add upgrader options Oct 5, 2022
@achingbrain
Copy link
Member

achingbrain commented Oct 6, 2022 •
edited
Loading

I don't know why GH isn't showing the actions that have run on this PR but the build is failing. Can you please ensure you can run the build, lint and test npm scripts successfully.

@ckousik
Copy link
Contributor Author

ckousik commented Oct 6, 2022

@achingbrain that is an older commit. I can run build, lint, and test successfully.

@ckousik
feat!: add upgrader options
1ea2e47 
BREAKING CHANGE: the return type of StreamMuxer.newStream can now return a promise

async new stream

fix mocks

Update packages/interface-transport/package.json

Co-authored-by: Marco Munizaga <marco@marcopolo.io>

test

fix versioning

fix lint
@ckousik ckousik force-pushed the ckousik/upgrader-options branch from edfc77c to 1ea2e47 Compare October 6, 2022 10:15
@ckousik
Copy link
Contributor Author

ckousik commented Oct 6, 2022

@achingbrain Could you merge this? I don't have permissions to merge.

@achingbrain
Copy link
Member

Yes, I was just waiting for CI to be green.

@achingbrain
Copy link
Member

It's here, still not sure why it doesn't appear on this PR: https://github.com/libp2p/js-libp2p-interfaces/actions/runs/3197326618

@achingbrain achingbrain merged commit c502b66 into libp2p:master Oct 6, 2022
github-actions bot pushed a commit that referenced this pull request Oct 6, 2022
@semantic-release-bot
chore(release): 3.0.0 [skip ci]
e7fc79e 
## [@libp2p/interface-stream-muxer-v3.0.0](https://github.com/libp2p/js-libp2p-interfaces/compare/@libp2p/interface-stream-muxer-v2.0.2...@libp2p/interface-stream-muxer-v3.0.0) (2022-10-06)

### ⚠ BREAKING CHANGES

* the return type of StreamMuxer.newStream can now return a promise

Co-authored-by: Marco Munizaga <marco@marcopolo.io>

### Features

* add upgrader options ([#290](#290)) ([c502b66](c502b66))
@github-actions
Copy link

github-actions bot commented Oct 6, 2022

🎉 This PR is included in version @libp2p/interface-stream-muxer-v3.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this pull request Oct 6, 2022
@semantic-release-bot
chore(release): 5.0.0 [skip ci]
23aead4 
## [@libp2p/interface-stream-muxer-compliance-tests-v5.0.0](https://github.com/libp2p/js-libp2p-interfaces/compare/@libp2p/interface-stream-muxer-compliance-tests-v4.0.0...@libp2p/interface-stream-muxer-compliance-tests-v5.0.0) (2022-10-06)

### ⚠ BREAKING CHANGES

* the return type of StreamMuxer.newStream can now return a promise

Co-authored-by: Marco Munizaga <marco@marcopolo.io>

### Features

* add upgrader options ([#290](#290)) ([c502b66](c502b66))

### Dependencies

* update sibling dependencies ([66b4993](66b4993))
@github-actions
Copy link

github-actions bot commented Oct 6, 2022

🎉 This PR is included in version @libp2p/interface-stream-muxer-compliance-tests-v5.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this pull request Oct 6, 2022
@semantic-release-bot
chore(release): 2.0.0 [skip ci]
65e61c2 
## [@libp2p/interface-transport-v2.0.0](https://github.com/libp2p/js-libp2p-interfaces/compare/@libp2p/interface-transport-v1.0.4...@libp2p/interface-transport-v2.0.0) (2022-10-06)

### ⚠ BREAKING CHANGES

* the return type of StreamMuxer.newStream can now return a promise

Co-authored-by: Marco Munizaga <marco@marcopolo.io>

### Features

* add upgrader options ([#290](#290)) ([c502b66](c502b66))

### Dependencies

* update sibling dependencies ([66b4993](66b4993))
* update sibling dependencies ([5de9728](5de9728))
@github-actions
Copy link

github-actions bot commented Oct 6, 2022

🎉 This PR is included in version @libp2p/interface-transport-v2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this pull request Oct 6, 2022
@semantic-release-bot
chore(release): 6.0.0 [skip ci]
7eef6a7 
## [@libp2p/interface-mocks-v6.0.0](https://github.com/libp2p/js-libp2p-interfaces/compare/@libp2p/interface-mocks-v5.1.0...@libp2p/interface-mocks-v6.0.0) (2022-10-06)

### ⚠ BREAKING CHANGES

* the return type of StreamMuxer.newStream can now return a promise

Co-authored-by: Marco Munizaga <marco@marcopolo.io>

### Features

* add upgrader options ([#290](#290)) ([c502b66](c502b66))

### Dependencies

* update sibling dependencies ([0fae3ee](0fae3ee))
* update sibling dependencies ([8a89a05](8a89a05))
* update sibling dependencies ([66b4993](66b4993))
@github-actions
Copy link

github-actions bot commented Oct 6, 2022

🎉 This PR is included in version @libp2p/interface-mocks-v6.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@p-shahi p-shahi mentioned this pull request Nov 10, 2022
Closed
4 tasks
@mpetrunic mpetrunic mentioned this pull request Nov 15, 2022
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MarcoPolo MarcoPolo MarcoPolo approved these changes

@achingbrain achingbrain achingbrain approved these changes

@wemeetagain wemeetagain Awaiting requested review from wemeetagain

@mpetrunic mpetrunic Awaiting requested review from mpetrunic

Assignees
No one assigned
Labels
released
Projects
js-libp2p
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ckousik @BigLep @MarcoPolo @mpetrunic @achingbrain