Bugs I'd like to fix

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Bugs I'd like to fix

Michael Baehr
I'd like to contribute to Groovy to help out.

As a start, Jochen mentioned the following bugs for me to look at:

<a href="http://jira.codehaus.org/browse/GROOVY-1114" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)"> http://jira.codehaus.org/browse/GROOVY-1114
I guess it's meant to be "... like String.split(/\W/), not String.split(/\w/);

<a href="http://jira.codehaus.org/browse/GROOVY-1126" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">http://jira.codehaus.org/browse/GROOVY-1126
Understood. I think eachObject(InputStream, Closure) should be implemented as well. Ok?

<a href="http://jira.codehaus.org/browse/GROOVY-1236" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)"> http://jira.codehaus.org/browse/GROOVY-1236
Understood

<a href="http://jira.codehaus.org/browse/GROOVY-1132" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)"> http://jira.codehaus.org/browse/GROOVY-1132
string.eachMatch(pattern_str) { match ->
found += match[0] + ' '
}
// but
(string=~ pattern_str).each { match ->
found += match + ' '
}
=> match vs match[] handling inconsistent
Ok, eachMatch calls the closure different depending on the number of parameters the closure accepts, each always calls the closure with an array. Each should make the same distinction. Correct?

<a href="http://jira.codehaus.org/browse/GROOVY-1131" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)"> http://jira.codehaus.org/browse/GROOVY-1131
public static Number next(Character self) { // Number/Character math ???
public static Number previous(Character self) {
++ and – should be closed under their type.
Guess that means that next and previous on Character should return Character. Right now, next and previous delegate to plus and minus respectively, but here there is the same logic - both plus and minus on Character return Number.
I'm not sure what to do here, please advise!
<a href="http://jira.codehaus.org/browse/GROOVY-1132" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">

Except for the last issue, I have fixes ready. I'll look at the test case policy to come up with unit tests for the new methods.

Can I send one single patch fixing all problems as it's all happening in the same class? And where should I send it?

If you have any more hints, comments and advice for a newcomer, please let me know!

Michael
Reply | Threaded
Open this post in threaded view
|

Re: Bugs I'd like to fix

Jochen Theodorou
Michael Baehr schrieb:
> I'd like to contribute to Groovy to help out.
>
> As a start, Jochen mentioned the following bugs for me to look at:
>
> http://jira.codehaus.org/browse/GROOVY-1114
> I guess it's meant to be "... like String.split(/\W/), not
> String.split(/\w/);

If we want basically the same as with tokenize, then \s+ might be
useful. \w would remove numbers I guess.

> http://jira.codehaus.org/browse/GROOVY-1126 
> <http://jira.codehaus.org/browse/GROOVY-1126>
> Understood. I think eachObject(InputStream, Closure) should be
> implemented as well. Ok?

As ObjectinputStream does behave sometimes a little strange I feel a bit
bad about doing this.. so I would say no.

> http://jira.codehaus.org/browse/GROOVY-1236
> Understood
>
> http://jira.codehaus.org/browse/GROOVY-1132
> string.eachMatch(pattern_str) { match ->
> found += match[0] + ' '
> }
> // but
> (string=~ pattern_str).each { match ->
> found += match + ' '
> }
> => match vs match[] handling inconsistent
> Ok, eachMatch calls the closure different depending on the number of
> parameters the closure accepts, each always calls the closure with an
> array. Each should make the same distinction. Correct?

I would say yes.

> http://jira.codehaus.org/browse/GROOVY-1131
> public static Number next(Character self) { // Number/Character math ???
> public static Number previous(Character self) {
> ++ and – should be closed under their type.
> Guess that means that next and previous on Character should return
> Character. Right now, next and previous delegate to plus and minus
> respectively, but here there is the same logic - both plus and minus on
> Character return Number.
> I'm not sure what to do here, please advise!

Sorry, please forget this issue, I will move it to 1.1. I don't want to
invalidate Dierks book with this.

> <http://jira.codehaus.org/browse/GROOVY-1132>
> Except for the last issue, I have fixes ready. I'll look at the test
> case policy to come up with unit tests for the new methods.
 >
> Can I send one single patch fixing all problems as it's all happening in
> the same class? And where should I send it?

It is always a problem to keep track on all the issues... so it would be
better to have a seperate patch for each.. sorry for the work ;) Attach
the patch to the issue as file.

> If you have any more hints, comments and advice for a newcomer, please
> let me know!

looking at the already written test cases does help. keep exceptions in
mind when doing something with streams. File#eachObject for example
should close the stream in case of errors. More hints? Ah yes, please
don't be sad if we decline a patch even thoguh it solves the problem. If
so, then it is because we think the patch has bad design or will create
more errors later. If you keep contact we can surely find a very good
solution. If your patch is not applied on the same day, please don't be
sad. If it isn't applied after some days, please get upset and tell us,
something went wrong then. One more, if you made some good patches then
we will give you rights to commit directly and then can apply the
patches yourself - no wait time, direct fun. But even then you can't
change all you want. Bigger changes need discussion. And regardless the
rights to commit, every change causing failing tests needs discussion
too. It is very possible that you had much work then and in the end we
decide not to apply the patch, again please don't be sad. Sometimes
something is looking very innocent and then it goes wild.

bye blackdrag

--
Jochen Theodorou
Groovy Tech Lead
http://blackdragsview.blogspot.com/

---------------------------------------------------------------------
To unsubscribe from this list please visit:

    http://xircles.codehaus.org/manage_email

Reply | Threaded
Open this post in threaded view
|

Re: Bugs I'd like to fix

Michael Baehr
Don't worry, I won't be sad if my patches get rejected - it's going to take quite some time to learn about the internals of Groovy.

I'm the sole developer of a Java library for a couple of years now, it has been my baby from the beginning.

And still it happens that I change a single line and a lot of bad things are starting to happen ...

Ok, as you (understandably) prefer to get single patches, I can only submit them one at a time as they are all in the same file.

I'll try to find the most innocent looking one and will start with that one - should happen tomorrow!

cya

Michael

On 9/9/06, Jochen Theodorou <[hidden email]> wrote:
Michael Baehr schrieb:
> I'd like to contribute to Groovy to help out.
>
> As a start, Jochen mentioned the following bugs for me to look at:
>
> http://jira.codehaus.org/browse/GROOVY-1114
> I guess it's meant to be "... like String.split(/\W/), not
> String.split(/\w/);

If we want basically the same as with tokenize, then \s+ might be
useful. \w would remove numbers I guess.

> http://jira.codehaus.org/browse/GROOVY-1126
> <http://jira.codehaus.org/browse/GROOVY-1126 >
> Understood. I think eachObject(InputStream, Closure) should be
> implemented as well. Ok?

As ObjectinputStream does behave sometimes a little strange I feel a bit
bad about doing this.. so I would say no.

> http://jira.codehaus.org/browse/GROOVY-1236
> Understood
>
> http://jira.codehaus.org/browse/GROOVY-1132
> string.eachMatch(pattern_str) { match ->
> found += match[0] + ' '
> }
> // but
> (string=~ pattern_str).each { match ->
> found += match + ' '
> }
> => match vs match[] handling inconsistent
> Ok, eachMatch calls the closure different depending on the number of
> parameters the closure accepts, each always calls the closure with an
> array. Each should make the same distinction. Correct?

I would say yes.

> http://jira.codehaus.org/browse/GROOVY-1131
> public static Number next(Character self) { // Number/Character math ???
> public static Number previous(Character self) {
> ++ and – should be closed under their type.
> Guess that means that next and previous on Character should return
> Character. Right now, next and previous delegate to plus and minus
> respectively, but here there is the same logic - both plus and minus on
> Character return Number.
> I'm not sure what to do here, please advise!

Sorry, please forget this issue, I will move it to 1.1. I don't want to
invalidate Dierks book with this.

> < http://jira.codehaus.org/browse/GROOVY-1132>
> Except for the last issue, I have fixes ready. I'll look at the test
> case policy to come up with unit tests for the new methods.
>
> Can I send one single patch fixing all problems as it's all happening in
> the same class? And where should I send it?

It is always a problem to keep track on all the issues... so it would be
better to have a seperate patch for each.. sorry for the work ;) Attach
the patch to the issue as file.

> If you have any more hints, comments and advice for a newcomer, please
> let me know!

looking at the already written test cases does help. keep exceptions in
mind when doing something with streams. File#eachObject for example
should close the stream in case of errors. More hints? Ah yes, please
don't be sad if we decline a patch even thoguh it solves the problem. If
so, then it is because we think the patch has bad design or will create
more errors later. If you keep contact we can surely find a very good
solution. If your patch is not applied on the same day, please don't be
sad. If it isn't applied after some days, please get upset and tell us,
something went wrong then. One more, if you made some good patches then
we will give you rights to commit directly and then can apply the
patches yourself - no wait time, direct fun. But even then you can't
change all you want. Bigger changes need discussion. And regardless the
rights to commit, every change causing failing tests needs discussion
too. It is very possible that you had much work then and in the end we
decide not to apply the patch, again please don't be sad. Sometimes
something is looking very innocent and then it goes wild.

bye blackdrag

--
Jochen Theodorou
Groovy Tech Lead
http://blackdragsview.blogspot.com/

---------------------------------------------------------------------
To unsubscribe from this list please visit:

    http://xircles.codehaus.org/manage_email


Reply | Threaded
Open this post in threaded view
|

Re: Bugs I'd like to fix

Jochen Theodorou
Michael Baehr schrieb:
> Don't worry, I won't be sad if my patches get rejected - it's going to
> take quite some time to learn about the internals of Groovy.
>
> I'm the sole developer of a Java library for a couple of years now, it
> has been my baby from the beginning.

ah yes, what is it?

> And still it happens that I change a single line and a lot of bad things
> are starting to happen ...
>
> Ok, as you (understandably) prefer to get single patches, I can only
> submit them one at a time as they are all in the same file.
>
> I'll try to find the most innocent looking one and will start with that
> one - should happen tomorrow!

thx very much!!

btw, cleaning up the bugtracker and writing documentation is also a very
big help. But no nice work - I know.

bye blackdrag

--
Jochen Theodorou
Groovy Tech Lead
http://blackdragsview.blogspot.com/

---------------------------------------------------------------------
To unsubscribe from this list please visit:

    http://xircles.codehaus.org/manage_email

Reply | Threaded
Open this post in threaded view
|

Re: Bugs I'd like to fix

Michael Baehr


On 9/9/06, Jochen Theodorou <[hidden email]> wrote:
Michael Baehr schrieb:
> Don't worry, I won't be sad if my patches get rejected - it's going to
> take quite some time to learn about the internals of Groovy.
>
> I'm the sole developer of a Java library for a couple of years now, it
> has been my baby from the beginning.

ah yes, what is it?

Closed source, unfortunately. Well, not so unfortunate, as I'm making a living with it ...

It's about "personalizing print documents" - technically its about converting document templates into pdf documents ready for high-quality printing. Sounds simple, but it supports highly dynamic layout rules, contains a almost complete pdf and font renderer and a lot of other stuff - about 150000 lines of code so far.
 
> And still it happens that I change a single line and a lot of bad things
> are starting to happen ...
>
> Ok, as you (understandably) prefer to get single patches, I can only
> submit them one at a time as they are all in the same file.
>
> I'll try to find the most innocent looking one and will start with that
> one - should happen tomorrow!

thx very much!!

btw, cleaning up the bugtracker and writing documentation is also a very
big help. But no nice work - I know.

Ah, yes, should work on some more documentation for my own stuff, too ...

We'll see, I just want to help to speed up things a little bit, so I'm also willing to do some dirty work - though I have enough other things to do myself.

And when I'll return to Jamaica in about two weeks, I wanted to look for a new girlfriend - well, I guess I'll make Groovy my girlfriend for the time being ... :-/

Michael