search

memorysafety / river

This repository is the home of the River reverse proxy application, based on the pingora library from Cloudflare.
https://www.memorysafety.org/initiative/reverse-proxy/
Apache License 2.0
1.72k stars 101 forks source link

Update release.yml to remove extraneous pieces from the "install" command #32

Closed bengaywins closed 4 months ago

bengaywins commented 5 months ago

Remove extraneous proto option and also don't specify TLS version to ensure ability to use 1.3 that is already available.

  1. There's no need to specify the proto option if you're specifying https in the URL
  2. GitHub already supports TLS 1.3. Let's not pin this to a TLS version. Allow cURL to properly use TLS 1.3 if support has been compiled in for their distro.
jamesmunns commented 5 months ago

Hi @bengaywins, thanks for the PR!

These values come from cargo-dist: https://github.com/axodotdev/cargo-dist, this action wasn't hand written.

In general, I'd prefer to keep it "stock" as I believe cargo-dist is capable of updating the script.

I'll leave this open, but I'd appreciate if you report this upstream cargo-dist to discuss first.

jamesmunns commented 5 months ago

It might be worth chiming in on https://github.com/axodotdev/cargo-dist/issues/457

bengaywins commented 5 months ago

Cool, I just created https://github.com/axodotdev/cargo-dist/pull/978 to fix it upstream.

jamesmunns commented 4 months ago

Closing this as per https://github.com/axodotdev/cargo-dist/issues/457#issuecomment-2086593505, as this appears to be essentially "require >= TLS 1.2", which seems reasonable to me.

Please feel free to reopen if the upstream opinion changes or if this observation is incorrect.