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

src: use Check instead of FromJust in QuicStream #33909

Closed
wants to merge 1 commit into from
Closed

src: use Check instead of FromJust in QuicStream #33909

wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Jun 16, 2020

This commit uses Maybe::Check() instead of Maybe::FromJust() as the
return value is not used.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@danbev
src: use Check instead of FromJust in QuicStream
3e613bb 
This commit uses Maybe::Check() instead of Maybe::FromJust() as the
return value is not used.
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jun 16, 2020
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31895/

@jasnell
Copy link
Member

jasnell commented Jun 16, 2020 •
edited
Loading

Note: Until we have CI jobs enabled that set the --experimental-quic configure flag, CI will need to be run manually on these PRs. Refs: nodejs/build#2353

@jasnell jasnell added experimental Issues and PRs related to experimental features. quic Issues and PRs related to the QUIC implementation / HTTP/3. labels Jun 16, 2020
@jasnell

This comment has been minimized.

Sign in to view
@richardlau

This comment has been minimized.

Sign in to view
@danbev
Copy link
Contributor Author

danbev commented Jun 17, 2020

Until we have CI jobs enabled that set the --experimental-quic configure flag, CI will need to be run manually on these PRs

I've never run a CI job manually, specifying the configuration parameters, could you tell me how that is done?

@richardlau
Copy link
Member

Until we have CI jobs enabled that set the --experimental-quic configure flag, CI will need to be run manually on these PRs

I've never run a CI job manually, specifying the configuration parameters, could you tell me how that is done?

@danbev :
image

@danbev
Copy link
Contributor Author

danbev commented Jun 17, 2020

@richardlau Thanks!

@jasnell jasnell added the fast-track PRs that do not need to wait for 48 hours to land. label Jun 17, 2020
@jasnell
Copy link
Member

jasnell commented Jun 17, 2020

@nodejs/quic ... fast track?

jasnell pushed a commit that referenced this pull request Jun 17, 2020
@danbev @jasnell
quic: use Check instead of FromJust in QuicStream
4b70f95 
This commit uses Maybe::Check() instead of Maybe::FromJust() as the
return value is not used.

PR-URL: #33909
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Jun 17, 2020

Landed in 4b70f95

@jasnell jasnell closed this Jun 17, 2020
@danbev danbev deleted the quic_stream_check branch June 18, 2020 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. experimental Issues and PRs related to experimental features. fast-track PRs that do not need to wait for 48 hours to land. quic Issues and PRs related to the QUIC implementation / HTTP/3.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@danbev @nodejs-github-bot @jasnell @richardlau @addaleax @codebytere