Skip to content
This repository has been archived by the owner on Aug 11, 2020. It is now read-only.

doc: add --experimental-quic in the doc #175

Closed
wants to merge 1 commit into from
Closed

doc: add --experimental-quic in the doc #175

wants to merge 1 commit into from

Conversation

trivikr
Copy link
Member

@trivikr trivikr commented Oct 16, 2019

Refs: #174
Refs: #83

Checklist

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

I assume the idea is that this will not be merged into the main repo when the PR is opened? :)

Also, @jasnell probably knows, but why do we use a build flag rather than a runtime flag?

@jasnell
Copy link
Member

jasnell commented Oct 16, 2019

Currently the build flag does not gate the modifications to openssl but I actually want them to. The reason I went with a build flag for now is so we can land in master soon without worrying too much about the still in flux changes to the openssl patches and the addition of the two new dependency libraries. There's also the long standing and still unresolved question about whether or not it is ok to introduce new unscoped top level modules (e.g. require('quic')) and the fact that a module called quic already exists in npm. Because of all that, I figured a build flag was generally safer for now. The goal would be to remove the flag as soon as possible and not even require a runtime flag, just have require('quic') emit the experimental warning while it is still experimental.

@jasnell
Copy link
Member

jasnell commented Oct 16, 2019

Rather than make this change, could we create a pinned issue that includes the build instructions?

@trivikr
Copy link
Member Author

trivikr commented Oct 16, 2019

I assume the idea is that this will not be merged into the main repo when the PR is opened?

Yup, the idea is to revert this change before merging with Node.js core

Rather than make this change, could we create a pinned issue that includes the build instructions?

+1 for creating a pinned issue

@trivikr
Copy link
Member Author

trivikr commented Oct 17, 2019

A pinned issue was created at #183

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants