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

fix: stream close/reset #823

Merged
merged 2 commits into from
Oct 24, 2023
Merged

fix: stream close/reset #823

merged 2 commits into from
Oct 24, 2023

Conversation

richard-ramos
Copy link
Member

Description

Renames the s variable to stream, and uses stream.Close() for the success path, and stream.Reset() for paths with errors

@status-im-auto
Copy link

status-im-auto commented Oct 22, 2023 •
edited
Loading

Jenkins Builds

Click to see older builds (18)
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ a31034c #1 2023-10-22 01:39:19 ~1 min nix-flake 📄log
✔️ a31034c #1 2023-10-22 01:39:46 ~2 min linux 📦deb
✔️ a31034c #1 2023-10-22 01:40:43 ~3 min tests 📄log
✔️ a31034c #1 2023-10-22 01:40:56 ~3 min tests 📄log
✔️ a31034c #1 2023-10-22 01:41:49 ~4 min android 📦tgz
✔️ a31034c #1 2023-10-22 01:42:48 ~5 min ios 📦tgz
✔️ c48543c #2 2023-10-24 16:30:24 ~4 min ios 📦tgz
✔️ c48543c #2 2023-10-24 16:32:18 ~5 min linux 📦deb
✔️ c48543c #2 2023-10-24 16:32:48 ~6 min nix-flake 📄log
✔️ c48543c #2 2023-10-24 16:33:17 ~6 min android 📦tgz
✔️ c48543c #2 2023-10-24 16:35:06 ~8 min tests 📄log
✔️ c48543c #2 2023-10-24 16:35:34 ~9 min tests 📄log
✔️ c829309 #3 2023-10-24 20:03:41 ~1 min nix-flake 📄log
✔️ c829309 #3 2023-10-24 20:03:42 ~1 min tests 📄log
✔️ c829309 #3 2023-10-24 20:04:06 ~2 min linux 📦deb
✔️ c829309 #3 2023-10-24 20:05:08 ~3 min tests 📄log
✔️ c829309 #3 2023-10-24 20:05:29 ~3 min android 📦tgz
✔️ c829309 #3 2023-10-24 20:05:45 ~3 min ios 📦tgz
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 83c8d18 #4 2023-10-24 20:08:54 ~1 min nix-flake 📄log
✔️ 83c8d18 #4 2023-10-24 20:09:18 ~2 min linux 📦deb
✔️ 83c8d18 #4 2023-10-24 20:09:19 ~2 min tests 📄log
✔️ 83c8d18 #4 2023-10-24 20:10:04 ~3 min android 📦tgz
✔️ 83c8d18 #4 2023-10-24 20:10:34 ~3 min tests 📄log
✔️ 83c8d18 #4 2023-10-24 20:10:56 ~3 min ios 📦tgz
✔️ 04487c3 #5 2023-10-24 21:45:27 ~1 min linux 📦deb
✔️ 04487c3 #5 2023-10-24 21:46:12 ~1 min tests 📄log
✔️ 04487c3 #5 2023-10-24 21:46:17 ~1 min nix-flake 📄log
✔️ 04487c3 #5 2023-10-24 21:46:41 ~2 min tests 📄log
✔️ 04487c3 #5 2023-10-24 21:47:48 ~3 min android 📦tgz
✔️ 04487c3 #5 2023-10-24 21:48:02 ~3 min ios 📦tgz

chaitanyaprem
chaitanyaprem reviewed Oct 23, 2023
View reviewed changes
Copy link
Collaborator

@chaitanyaprem chaitanyaprem left a comment

Choose a reason for hiding this comment

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

Looks good.
Left few comments.

waku/v2/protocol/filter/client.go Show resolved Hide resolved
waku/v2/protocol/filter/client.go Show resolved Hide resolved
waku/v2/protocol/filter/server.go Show resolved Hide resolved
@richard-ramos richard-ramos mentioned this pull request Oct 24, 2023
Closed
richard-ramos added a commit that referenced this pull request Oct 24, 2023
@richard-ramos
fix: stream close/reset (#823)
7415c5d
chaitanyaprem
chaitanyaprem approved these changes Oct 24, 2023
View reviewed changes
Copy link
Collaborator

@chaitanyaprem chaitanyaprem left a comment

Choose a reason for hiding this comment

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

LGTM

@richard-ramos richard-ramos merged commit 94f18c5 into master Oct 24, 2023
9 of 10 checks passed
@richard-ramos richard-ramos deleted the stream-close-reset branch October 24, 2023 21:48
richard-ramos added a commit that referenced this pull request Oct 24, 2023
@richard-ramos
fix: stream close/reset (#823)
fa51d10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chaitanyaprem chaitanyaprem chaitanyaprem approved these changes

@harsh-98 harsh-98 Awaiting requested review from harsh-98

Assignees

@richard-ramos richard-ramos

Labels
None yet
Projects
Waku
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@richard-ramos @status-im-auto @chaitanyaprem