Closed wuvs closed 7 years ago
Thanks for the contribution! Please ensure your commits follow our style guide. This code will be tested once a Deis maintainer reviews it.
Merging #341 into master will not change coverage. The diff coverage is
100%
.
@@ Coverage Diff @@
## master #341 +/- ##
=======================================
Coverage 56.47% 56.47%
=======================================
Files 6 6
Lines 425 425
=======================================
Hits 240 240
Misses 159 159
Partials 26 26
Impacted Files | Coverage Δ | |
---|---|---|
nginx/config.go | 56.86% <ø> (ø) |
:arrow_up: |
model/model.go | 49.18% <100%> (ø) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 7988fef...5894ea2. Read the comment docs.
Requires #340
Do we have any concerns about turning nginx's initial implementation of TLS v1.3 on? I'd like to get @krancour's review here.
@wuvs I have to admit to not being too in-the-know (yet) about TLS 1.3. Am I mistaken in saying it is still in a draft? I think we have some minor concern about unconditionally enabling this. Can you speak at all to the relative risk or safety of this?
Since we still have open questions about nginx and TLS v1.3, and we're trying to finalize Workflow v2.14 today, I'm going to defer this to the next release.
@krancour you are right it's actually still in a draft (openssl draft-18 branch) and should be released in openssl 1.1.1. so i think you actually can close this PR as you would have to build your own router image anyway to test tls1.3
Thanks for the update @wuvs. We'll be happy for you to re-open this when the time comes.
http://nginx.org/en/CHANGES