Re: [Patch] address@hidden Updated patch

[Alex Vong]

Hello,

I think match-lambda is not documented in the guile manual (it only
appears in an example). While its usage is more or less the same as in
racket, I think it should be documented. What is your idea?

Thanks,
Alex

On 06/06/2016, Andy Wingo <address@hidden> wrote:
> Hi :)
>
> Looking good!  I have some style nits :)
>
> On Fri 03 Jun 2016 19:39, Matthew Jordan <address@hidden>
> writes:
>
>> +               ;; Removing net/ tests
>> +               (for-each
>> +                (lambda (srcfile)
>> +                  (let ((srcfile (string-append "net/" srcfile)))
>> +                  (if (file-exists? srcfile)
>> +                      (delete-file srcfile))))
>> +                '("multicast_test.go" "parse_test.go" "port_test.go"))
>
> Either these files exist or they do not.  Better to just have three
> unconditional delete-file invocations.
>
>> +
>> +               ;; Add libgcc to runpath
>> +               (substitute* "cmd/go/build.go"
>> +                 (("cgoldflags := \\[\\]string\\{\\}")
>> +                  (string-append "cgoldflags := []string{"
>> +                                 "\"-rpath=" gcclib "\""
>> +                                 "}"))
>> +                 (("ldflags := buildLdflags")
>> +                  (string-append
>> +                   "ldflags := buildLdflags\n"
>> +                   "ldflags = append(ldflags, \"-r\")\n"
>> +                   "ldflags = append(ldflags, \"" gcclib "\")\n")))
>> +
>> +               (substitute* "os/os_test.go"
>> +                 (("/usr/bin") (getcwd))
>> +                 (("/bin/pwd") (which "pwd")))
>> +
>> +               ;; Disable failing tests
>> +               (for-each
>> +                (lambda (srcfile)
>> +                  (substitute* (car srcfile)
>> +                    (((cdr srcfile) all) (string-append all
>> "return\n"))))
>
> Is this a nice way to disable tests?  I dunno.  Why not change the test
> name from TestFoo to DisabledTestFoo ?  Then it would not get run.
>
> Also let's avoid car and cdr, and instead use match-lambda:
>
>   (match-lambda
>    ((file . pattern)
>     ...))
>
> But let's also avoid dotted pairs in bigger literal data structures;
> it's ugly when there's lots of punctuation around.  So if you change the
> lines to be like
>
>   ("net/net_test.go" ".+TestShutdownUnix.+")
>
> you can
>
>   (match-lambda
>    ((file pattern)
>     ...))
>
> Note the lack of " . ".
>
>> +                (list
>> +                 '("net/net_test.go" . ".+TestShutdownUnix.+")
>> +                 '("net/dial_test.go" . ".+TestDialTimeout.+")
>
> Here make the whole list a literal instead of calling `list' on a list
> of quoted datums:
>
>   '(("net/net_test.go" ".+TestHostname.+")
>     ...)
>
> Cheers,
>
> Andy
>
>