Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com, rsc@golang.org), I'd like you to review this change to https://code.google.com/p/go/
11 years, 7 months ago
(2012-09-22 09:43:58 UTC)
#1
Fixes issue 4124 Also, I think you should include the test code from issue 4124 ...
11 years, 7 months ago
(2012-09-22 09:59:51 UTC)
#3
Fixes issue 4124
Also, I think you should include the test code from issue 4124 in
addition to your reflect example.
On Sat, Sep 22, 2012 at 7:45 PM, <daniel.morsing@gmail.com> wrote:
> I think I got this right, but I'm very unsure. Could everyone please
> that a closer look than normal?
>
>
> http://codereview.appspot.com/6543057/
On 2012/09/22 09:59:51, dfc wrote: > Fixes issue 4124 Whoops. Added now > > Also, ...
11 years, 7 months ago
(2012-09-22 11:18:32 UTC)
#4
On 2012/09/22 09:59:51, dfc wrote:
> Fixes issue 4124
Whoops. Added now
>
> Also, I think you should include the test code from issue 4124 in
> addition to your reflect example.
I tried, but I'd have to use compiledir and that can't handle compilations that
fails.
The test using reflect is nice, but can you also make a test checking the ...
11 years, 7 months ago
(2012-09-22 12:51:23 UTC)
#5
The test using reflect is nice, but can you also make a test checking
the behavior at compile time? Strictly speaking there's no guarantee
the two would be in sync. It will have to be a 2-file 'rundir' test
like the one you found before.
Russ
On Sat, Sep 22, 2012 at 2:51 PM, Russ Cox <rsc@golang.org> wrote: > The test ...
11 years, 7 months ago
(2012-09-22 14:26:31 UTC)
#6
On Sat, Sep 22, 2012 at 2:51 PM, Russ Cox <rsc@golang.org> wrote:
> The test using reflect is nice, but can you also make a test checking
> the behavior at compile time? Strictly speaking there's no guarantee
> the two would be in sync. It will have to be a 2-file 'rundir' test
> like the one you found before.
>
compiledir only checks that the compile is successful. Since I have to
do something like errorcheckdir, I think I have to change the test
framework in order to test that part of this change.
> compiledir only checks that the compile is successful. Since I have to > do ...
11 years, 7 months ago
(2012-09-22 15:03:08 UTC)
#7
> compiledir only checks that the compile is successful. Since I have to
> do something like errorcheckdir, I think I have to change the test
> framework in order to test that part of this change.
indeed. sorry about that. if you're going to do that could you also
add rundir while you're there (issue 4058)? thanks.
On Sat, Sep 22, 2012 at 5:03 PM, Russ Cox <rsc@golang.org> wrote: >> compiledir only ...
11 years, 7 months ago
(2012-09-22 15:30:32 UTC)
#8
On Sat, Sep 22, 2012 at 5:03 PM, Russ Cox <rsc@golang.org> wrote:
>> compiledir only checks that the compile is successful. Since I have to
>> do something like errorcheckdir, I think I have to change the test
>> framework in order to test that part of this change.
>
> indeed. sorry about that. if you're going to do that could you also
> add rundir while you're there (issue 4058)? thanks.
Will do. Do I need to add it to testlib as well?
On Saturday, September 22, 2012, Daniel Morsing wrote: > On Sat, Sep 22, 2012 at ...
11 years, 7 months ago
(2012-09-22 15:36:45 UTC)
#9
On Saturday, September 22, 2012, Daniel Morsing wrote:
> On Sat, Sep 22, 2012 at 5:03 PM, Russ Cox <rsc@golang.org <javascript:;>>
> wrote:
> > indeed. sorry about that. if you're going to do that could you also
> > add rundir while you're there (issue 4058)? thanks.
> Will do. Do I need to add it to testlib as well?
>
Please do so. test/run should still be able to do all the tests
(some tests are still runnable only by testlib)
Little postscript to this CL. Should we change the naming convention for files in the ...
11 years, 6 months ago
(2012-10-07 04:59:29 UTC)
#15
Little postscript to this CL. Should we change the naming convention for files
in the fixed bugs directory? I had to rename my tests twice because someone had
already taken the name.
I know of at least one test which got overwritten because of this problem, but I
can't find a reference to it.
On Sun, Oct 7, 2012 at 12:59 PM, <daniel.morsing@gmail.com> wrote: > Little postscript to this ...
11 years, 6 months ago
(2012-10-07 05:48:07 UTC)
#16
On Sun, Oct 7, 2012 at 12:59 PM, <daniel.morsing@gmail.com> wrote:
> Little postscript to this CL. Should we change the naming convention for
> files in the fixed bugs directory? I had to rename my tests twice
> because someone had already taken the name.
>
Yeah, the same happened to me twice.
I propose we use something like test/fixedissues/issueNNN.go.
>
> I know of at least one test which got overwritten because of this
> problem, but I can't find a reference to it.
>
http://codereview.appspot.com/6443100
Issue 6543057: code review 6543057: cmd/gc: Don't export embedded builtins
(Closed)
Created 11 years, 7 months ago by DMorsing
Modified 11 years, 6 months ago
Reviewers:
Base URL:
Comments: 0