- From: Matt Menke <notifications@github.com>
- Date: Tue, 04 Aug 2020 13:34:45 -0700
- To: whatwg/fetch <fetch@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <whatwg/fetch/pull/1063/review/461129969@github.com>
@MattMenke2 commented on this pull request. Thanks for the feedback, Anne! > @@ -2123,11 +2125,11 @@ clearly stipulates that <a>connections</a> are keyed on WebSocket saner --> -<h3 id=http-cache-partitions>HTTP cache partitions</h3> +<h3 id=http-state-partition>HTTP state partition key</h3> I was thinking "network" could be miscontrued to mean this was a per-network key (e.g., one key while at home, another key connected through Starbucks, etc), though suppose the same is true for "state". I've replaced "http state" with "network" everywhere. Of course, there's still the issue of potentially using this for things that aren't directly network related. > given <var>httpRequest</var>. + <li><p>Set <var>httpCache</var> to null. I assume we need to declare variables exist before conditionally setting them to something. If this were psuedo-Javascript, it's undefined at this point, not null. If this is psuedo-C, it's a compile failure. Anyhow, I've reverted this and added back a shortened HTTP cache partition section, per your other comment. > @@ -4796,8 +4806,8 @@ Range Requests</cite>. [[HTTP-RANGE]] However, this is not widely supported by b <li><p>Let <var>response</var> be null. - <li><p>Let <var>httpCache</var> be the result of <a>determining the HTTP cache partition</a>, given - <var>httpRequest</var>. + <li><p>Let <var>httpCache</var> be the result of <a>determining the HTTP state partition key</a>, + given <var>httpRequest</var>. This would be just as accurate if this were null-checking checking the network partition key, and I just renamed httpCache here to networkPartitionKey - I just forgot to rename the variable. That having been said, I've brought back the cache section, though it's pretty tiny now, and reverted this to directly reference the cache. That does seem less regression prone. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/whatwg/fetch/pull/1063#pullrequestreview-461129969
Received on Tuesday, 4 August 2020 20:34:58 UTC