You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by Myrna van Lunteren <m....@gmail.com> on 2005/05/16 22:32:48 UTC

[PATCH] test harness sed.java & readme

Hi,

>From looking at functionTests/harness/Sed.java after some trouble
reported because of directory names, I noticed there was a lot of old
- let's call it: - stuff that wasn't needed anymore. So, this patch
contains a slightly cleaned up file.

Also, I added a couple of small sections to the readme to:
- document jvmflags
- document that all test files, including .out etc. are accessed from
the CLASSPATH and thus ant all is needed
- document that directory names may not have colons or spaces in them.

I hope those additions are clear, if a committer could review & check
in the patch...

Thx,
Myrna

Re: [PATCH] test harness sed.java & readme

Posted by Andrew McIntyre <mc...@gmail.com>.
On 5/18/05, Myrna van Lunteren <m....@gmail.com> wrote:
> To check, you want me to:
> 1. make a separate patch for Sed.java without any formatting.
> 2. another one for the readme.txt.

Actually, no, I've already reviewed the patch and it's fine. It just
took me a while, and it would have been easier if there were not so
many diffs that were just formatting related. I'll commit what you've
already submitted as-is.

> 3. log a bug for the spaces not working & fix that with the readme.txt patch

Yes, please. (or I will :-)  )

> And of course I ran all tests. Should've said so.

I suspected as much, but I wanted to make sure. No need to submit a
separate patch.

andrew

Re: [PATCH] test harness sed.java & readme

Posted by Myrna van Lunteren <m....@gmail.com>.
To check, you want me to:
1. make a separate patch for Sed.java without any formatting.
2. another one for the readme.txt.
3. log a bug for the spaces not working & fix that with the readme.txt patch
?

I think Jeremy was planning on doing a whole formatting thing - so
I'll see if I can undo those changes & redo the others and let Jeremy
handle the formatting.

And of course I ran all tests. Should've said so.
Of course, now I have to run them again. Will post a new patch tomorrow. or so.

Myrna

On 5/18/05, Andrew McIntyre <mc...@gmail.com> wrote:
> 
> On May 17, 2005, at 3:51 PM, Dag H. Wanvik wrote:
> 
> >>>>>> "MvL" == Myrna van Lunteren <m....@gmail.com> wrote:
> >
> > MvL> From looking at functionTests/harness/Sed.java after some trouble
> > MvL> reported because of directory names, I noticed there was a lot of
> > old
> > MvL> - let's call it: - stuff that wasn't needed anymore. So, this
> > patch
> > MvL> contains a slightly cleaned up file.
> >
> > Would this also fix Derby-266? Or should that be closed and just
> > called out in docs, too?
> 
> It appears from the description of Derby-266 that this patch would fix
> that issue.
> 
> Myrna, a couple things:
> 
> Reviewing this patch is rather difficult because it includes a lot of
> format changes mixed in with code that has been altered or removed. I'm
> not against reformatting, Sed.java is a particularly bad offender, but
> formatting changes should be in a separate diff.
> 
> Also, you didn't mention it in your original mail whether or not you
> ran derbyall and whether or not it passed. And, a JIRA should probably
> be filed for the fact that the test harness can't run in a path that
> contains spaces.
> 
> andrew
> 
>

Re: [PATCH] test harness sed.java & readme

Posted by Andrew McIntyre <mc...@gmail.com>.
On May 17, 2005, at 3:51 PM, Dag H. Wanvik wrote:

>>>>>> "MvL" == Myrna van Lunteren <m....@gmail.com> wrote:
>
> MvL> From looking at functionTests/harness/Sed.java after some trouble
> MvL> reported because of directory names, I noticed there was a lot of 
> old
> MvL> - let's call it: - stuff that wasn't needed anymore. So, this 
> patch
> MvL> contains a slightly cleaned up file.
>
> Would this also fix Derby-266? Or should that be closed and just
> called out in docs, too?

It appears from the description of Derby-266 that this patch would fix 
that issue.

Myrna, a couple things:

Reviewing this patch is rather difficult because it includes a lot of 
format changes mixed in with code that has been altered or removed. I'm 
not against reformatting, Sed.java is a particularly bad offender, but 
formatting changes should be in a separate diff.

Also, you didn't mention it in your original mail whether or not you 
ran derbyall and whether or not it passed. And, a JIRA should probably 
be filed for the fact that the test harness can't run in a path that 
contains spaces.

andrew


Re: [PATCH] test harness sed.java & readme

Posted by "Dag H. Wanvik" <Da...@Sun.COM>.
Hi Myrna,

>>>>> "MvL" == Myrna van Lunteren <m....@gmail.com> wrote:

MvL> From looking at functionTests/harness/Sed.java after some trouble
MvL> reported because of directory names, I noticed there was a lot of old
MvL> - let's call it: - stuff that wasn't needed anymore. So, this patch
MvL> contains a slightly cleaned up file.

Would this also fix Derby-266? Or should that be closed and just
called out in docs, too?

Dag

Re: [PATCH] test harness sed.java & readme

Posted by Andrew McIntyre <mc...@gmail.com>.
On 5/16/05, Myrna van Lunteren <m....@gmail.com> wrote:
> From looking at functionTests/harness/Sed.java after some trouble
> reported because of directory names, I noticed there was a lot of old
> - let's call it: - stuff that wasn't needed anymore. So, this patch
> contains a slightly cleaned up file.

Committed, revision 170880.

andrew