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