Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(607)

Issue 4789042: code review 4789042: http: make serveFile redirects relative to work with St... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 8 months ago by andybalholm
Modified:
13 years, 7 months ago
Reviewers:
CC:
golang-dev, rsc, bradfitz, kevlar
Visibility:
Public.

Description

http: make serveFile redirects relative to work with StripPrefix serveFile was using absolute redirects, which didn't work under StripPrefix. Now it uses relative redirects.

Patch Set 1 #

Patch Set 2 : diff -r 2e7ece227c7d https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r 2e7ece227c7d https://go.googlecode.com/hg/ #

Patch Set 4 : diff -r 2e7ece227c7d https://go.googlecode.com/hg/ #

Patch Set 5 : diff -r 2e7ece227c7d https://go.googlecode.com/hg/ #

Patch Set 6 : diff -r 6a3ad528f59d https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+39 lines, -3 lines) Patch
M src/pkg/http/fs.go View 1 2 3 4 5 3 chunks +15 lines, -3 lines 0 comments Download
M src/pkg/http/fs_test.go View 1 2 3 4 1 chunk +24 lines, -0 lines 0 comments Download

Messages

Total messages: 15
andybalholm
Hello golang-dev@googlegroups.com (cc: bradfitz@golang.org, golang-dev@googlegroups.com, kevlar@google.com), I'd like you to review this change to https://go.googlecode.com/hg/
13 years, 8 months ago (2011-07-19 20:09:45 UTC) #1
bradfitz
I'm afraid the next CL involves parsing the response HTML and rewriting links. StripPrefix does ...
13 years, 8 months ago (2011-07-19 20:13:26 UTC) #2
rsc
No thanks. StripPrefix is about stripping a prefix, which it does admirably. That's *all* it ...
13 years, 8 months ago (2011-07-19 20:32:16 UTC) #3
andybalholm
On Jul 19, 2011, at 1:32 PM, Russ Cox wrote: > No thanks. > > ...
13 years, 8 months ago (2011-07-19 20:42:34 UTC) #4
bradfitz
On Tue, Jul 19, 2011 at 1:42 PM, Andy Balholm <andybalholm@gmail.com> wrote: > On Jul ...
13 years, 8 months ago (2011-07-19 20:45:11 UTC) #5
rsc
On Tue, Jul 19, 2011 at 16:42, Andy Balholm <andybalholm@gmail.com> wrote: > On Jul 19, ...
13 years, 8 months ago (2011-07-19 20:48:10 UTC) #6
andybalholm
Hello golang-dev@googlegroups.com, rsc@golang.org (cc: bradfitz@golang.org, golang-dev@googlegroups.com, kevlar@google.com), Please take another look.
13 years, 8 months ago (2011-07-19 21:43:49 UTC) #7
andybalholm
On Jul 19, 2011, at 1:45 PM, Brad Fitzpatrick wrote: > Why not just a ...
13 years, 8 months ago (2011-07-19 21:47:02 UTC) #8
andybalholm
On Jul 19, 2011, at 1:48 PM, Russ Cox wrote: > i suggest that the ...
13 years, 8 months ago (2011-07-19 22:01:21 UTC) #9
bradfitz
I'm in favor of making the redirect "./", but can this be done with less ...
13 years, 8 months ago (2011-07-19 22:44:20 UTC) #10
andybalholm
Hello golang-dev@googlegroups.com, rsc@golang.org, bradfitz@golang.org (cc: golang-dev@googlegroups.com, kevlar@google.com), Please take another look.
13 years, 8 months ago (2011-07-19 23:45:42 UTC) #11
andybalholm
Hello golang-dev@googlegroups.com, rsc@golang.org, bradfitz@golang.org (cc: golang-dev@googlegroups.com, kevlar@google.com), Please take another look.
13 years, 7 months ago (2011-07-23 15:13:35 UTC) #12
andybalholm
On 2011/07/19 22:44:20, bradfitz wrote: > I'm in favor of making the redirect "./", but ...
13 years, 7 months ago (2011-07-28 14:33:28 UTC) #13
bradfitz
*** Submitted as http://code.google.com/p/go/source/detail?r=acae1d69e7a0 *** http: make serveFile redirects relative to work with StripPrefix serveFile ...
13 years, 7 months ago (2011-07-28 18:44:17 UTC) #14
bradfitz
13 years, 7 months ago (2011-07-28 19:12:39 UTC) #15
LGTM

On Thu, Jul 28, 2011 at 7:33 AM, <andybalholm@gmail.com> wrote:

> On 2011/07/19 22:44:20, bradfitz wrote:
>
>> I'm in favor of making the redirect "./", but can this be done with
>>
> less
>
>> repetition?  And perhaps a test?
>>
>
> I've moved the redirect code into a new function, and written a test.
> Does anyone want to review the changes?
>
>
>
http://codereview.appspot.com/**4789042/<http://codereview.appspot.com/4789042/>
>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b