code review 4291069: http: modified perl cgi test to remove newline from env...

http: modified perl cgi test to remove newline from env variables

Fixes issue 1639

[royger, 2011-03-25 12:20:30]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[bradfitz, 2011-03-25 14:56:35]

http://codereview.appspot.com/4291069/diff/5001/src/pkg/http/cgi/testdata/tes...
File src/pkg/http/cgi/testdata/test.cgi (right):

http://codereview.appspot.com/4291069/diff/5001/src/pkg/http/cgi/testdata/tes...
src/pkg/http/cgi/testdata/test.cgi:33: print "env-$k=".do { (my $new_env =
$ENV{$k}) =~ s/[\n\r]//g; $new_env } . "\n";
Thanks for the fix!

I'd prefer if we didn't play Perl Golf here.  Could you burn a few more lines
here for future maintainers who might not know Perl as well?

my $clean_env = $ENV{$k};
$clean_env =~ s/[\n\r]//g;
print "env-$k=$clean_env\n";

[royger, 2011-03-25 15:04:26]

Hello golang-dev@googlegroups.com, bradfitzgo (cc: golang-dev@googlegroups.com),

Please take another look.

[royger, 2011-03-25 15:06:26]

Hello golang-dev@googlegroups.com, bradfitzgo (cc: golang-dev@googlegroups.com),

Please take another look.

[royger, 2011-03-25 15:08:19]

On 2011/03/25 14:56:35, bradfitzgo wrote:
>
http://codereview.appspot.com/4291069/diff/5001/src/pkg/http/cgi/testdata/tes...
> File src/pkg/http/cgi/testdata/test.cgi (right):
> 
>
http://codereview.appspot.com/4291069/diff/5001/src/pkg/http/cgi/testdata/tes...
> src/pkg/http/cgi/testdata/test.cgi:33: print "env-$k=".do { (my $new_env =
> $ENV{$k}) =~ s/[\n\r]//g; $new_env } . "\n";
> Thanks for the fix!
> 
> I'd prefer if we didn't play Perl Golf here.  Could you burn a few more lines
> here for future maintainers who might not know Perl as well?
> 
> my $clean_env = $ENV{$k};
> $clean_env =~ s/[\n\r]//g;
> print "env-$k=$clean_env\n";

I did it this way first, but then I through it will look more "professional" in
one line, sorry for the indentation problem.

[bradfitz, 2011-03-25 15:28:02]

Roger,

LGTM but before I can submit this, you need to be in the AUTHORS and
CONTRIBUTORS file.  See the Copyright section here:

     http://golang.org/doc/contribute.html#copyright

<http://golang.org/doc/contribute.html#copyright>You only have to do this
once and then future patches are easy.

- Brad

On Fri, Mar 25, 2011 at 8:08 AM, <royger@gmail.com> wrote:

> On 2011/03/25 14:56:35, bradfitzgo wrote:
>
>
>
http://codereview.appspot.com/4291069/diff/5001/src/pkg/http/cgi/testdata/tes...
>
>> File src/pkg/http/cgi/testdata/test.cgi (right):
>>
>
>
>
>
http://codereview.appspot.com/4291069/diff/5001/src/pkg/http/cgi/testdata/tes...
>
>> src/pkg/http/cgi/testdata/test.cgi:33: print "env-$k=".do { (my
>>
> $new_env =
>
>> $ENV{$k}) =~ s/[\n\r]//g; $new_env } . "\n";
>> Thanks for the fix!
>>
>
>  I'd prefer if we didn't play Perl Golf here.  Could you burn a few
>>
> more lines
>
>> here for future maintainers who might not know Perl as well?
>>
>
>  my $clean_env = $ENV{$k};
>> $clean_env =~ s/[\n\r]//g;
>> print "env-$k=$clean_env\n";
>>
>
> I did it this way first, but then I through it will look more
> "professional" in one line, sorry for the indentation problem.
>
>
>
> http://codereview.appspot.com/4291069/
>

[royger, 2011-03-25 16:15:02]

Done.

On 2011/03/25 15:28:02, bradfitzgo wrote:
> Roger,
> 
> LGTM but before I can submit this, you need to be in the AUTHORS and
> CONTRIBUTORS file.  See the Copyright section here:
> 
>      http://golang.org/doc/contribute.html#copyright
> 
> <http://golang.org/doc/contribute.html#copyright%3EYou only have to do this
> once and then future patches are easy.
> 
> - Brad
> 
> On Fri, Mar 25, 2011 at 8:08 AM, <mailto:royger@gmail.com> wrote:
> 
> > On 2011/03/25 14:56:35, bradfitzgo wrote:
> >
> >
> >
>
http://codereview.appspot.com/4291069/diff/5001/src/pkg/http/cgi/testdata/tes...
> >
> >> File src/pkg/http/cgi/testdata/test.cgi (right):
> >>
> >
> >
> >
> >
>
http://codereview.appspot.com/4291069/diff/5001/src/pkg/http/cgi/testdata/tes...
> >
> >> src/pkg/http/cgi/testdata/test.cgi:33: print "env-$k=".do { (my
> >>
> > $new_env =
> >
> >> $ENV{$k}) =~ s/[\n\r]//g; $new_env } . "\n";
> >> Thanks for the fix!
> >>
> >
> >  I'd prefer if we didn't play Perl Golf here.  Could you burn a few
> >>
> > more lines
> >
> >> here for future maintainers who might not know Perl as well?
> >>
> >
> >  my $clean_env = $ENV{$k};
> >> $clean_env =~ s/[\n\r]//g;
> >> print "env-$k=$clean_env\n";
> >>
> >
> > I did it this way first, but then I through it will look more
> > "professional" in one line, sorry for the indentation problem.
> >
> >
> >
> > http://codereview.appspot.com/4291069/
> >

[bradfitz, 2011-03-25 16:33:50]

*** Submitted as http://code.google.com/p/go/source/detail?r=11c0906fdf7d ***

http: modified perl cgi test to remove newline from env variables

Fixes issue 1639

R=golang-dev, bradfitzgo
CC=golang-dev
http://codereview.appspot.com/4291069

Committer: Brad Fitzpatrick <bradfitz@golang.org>