Issue 81 & 82 Implicit conversion to support arithmetic operations for BigDecimal types

4 views
Skip to first unread message

Jimmy Yuen Ho Wong

unread,
Jan 16, 2011, 10:34:04 PM1/16/11
to squeryl-contributors
https://github.com/max-l/Squeryl/issues#issue/81
https://github.com/max-l/Squeryl/issues#issue/82

As explained in the tests, this pull request is a patch for BigDecimal
arithmetic support in where clauses. Max is there a particular place
you don't understand?

Maxime Lévesque

unread,
Jan 17, 2011, 12:28:13 PM1/17/11
to squeryl-co...@googlegroups.com
I'm glad you ask Jimmy !

I've been procrastinating on pulling this because I have a hard time understanding the change,
I mean, I understand the intent, but I'd like to look systematically at each line and see what
happended to it.

I have a question for you : is it possible that git is chowing more changes than there are in reality :
  http://stackoverflow.com/questions/861995/is-it-possible-for-git-merge-to-ignore-line-ending-differences
?

Anyhow, I will print the diff and try to mentally digest it

https://github.com/wyuenho/Squeryl/commit/4d7f8aa0bf60dc047f677329f780b2d972f9905c


Thanks

Jimmy Yuen Ho Wong

unread,
Jan 18, 2011, 2:05:16 AM1/18/11
to squeryl-contributors
Well if you are referring to the github diff, that's the correct diff.
If you are talking about what's showing up on your git client, then I
have no idea because I'm on a Mac.

The diff shows that many lines is mainly because all the methods are
numbered sequentially, and I inserted the BigDecimal methods in the
middle of the big block. All the lines below it obviously had to be
incremented accordingly. I could have just put the BigDecimal methods
to the end of the block but, I was just following the convention you
seemed to have set up.

Anyway, I think what you can do is instead of pulling from github,
just checkout my fork and merge to your local copy on your own
machine. If you don't like it you can always revert.

Jimmy


On Jan 18, 1:28 am, Maxime Lévesque <maxime.leves...@gmail.com> wrote:
> I'm glad you ask Jimmy !
>
> I've been procrastinating on pulling this because I have a hard time
> understanding the change,
> I mean, I understand the intent, but I'd like to look systematically at each
> line and see what
> happended to it.
>
> I have a question for you : is it possible that git is chowing more changes
> than there are in reality :
>
> http://stackoverflow.com/questions/861995/is-it-possible-for-git-merg...
> ?
>
> Anyhow, I will print the diff and try to mentally digest it
>
> https://github.com/wyuenho/Squeryl/commit/4d7f8aa0bf60dc047f677329f78...
>
> Thanks

Maxime Lévesque

unread,
Jan 18, 2011, 9:14:25 PM1/18/11
to squeryl-co...@googlegroups.com

Jimmy, here is some background, the conversions were initially generated by this code :

https://github.com/wyuenho/Squeryl/blob/4d7f8aa0bf60dc047f677329f780b2d972f9905c/src/test/scala/org/squeryl/tests/labo/TypeArithmetic.scala

the idea is that combinatorically there are so many of them that the only way to have confidence
was to systematically generate them.

BigDecimal support was added later by Antti Tupurainen :

  https://github.com/max-l/Squeryl/commit/8c962d8fa5c34b4a3400028cad98429458349b30

he suffixed his additions with "bd", so he broke the numbering, but when one looks at the commit,
one can mentally grasp what happened.

I really hate to ask you this, but could you redo your commit by not changing other lines ?
(You can use whatever suffix, like Antti)
...It really hurts me to ask you this believe me, but to review the commit will be as hard as
to rewrite it and I would hate to rewrite it, since as your test case confirms, it looks like it's
doing the right thing...

I hope you will undersand ;-)

Thanks !

Jimmy Yuen Ho Wong

unread,
Jan 21, 2011, 2:18:50 AM1/21/11
to squeryl-contributors
Ah ok. I don't mind redoing the patch, as I basically have generated
it in Emacs anyway:) I'll get to it when I get home tonight. I'll just
add "bd" to the end of my additions.

Jimmy


On Jan 19, 10:14 am, Maxime Lévesque <maxime.leves...@gmail.com>
wrote:
> Jimmy, here is some background, the conversions were initially generated by
> this code :
>
> https://github.com/wyuenho/Squeryl/blob/4d7f8aa0bf60dc047f677329f780b...
>
> the idea is that combinatorically there are so many of them that the only
> way to have confidence
> was to systematically generate them.
>
> BigDecimal support was added later by Antti Tupurainen :
>
> https://github.com/max-l/Squeryl/commit/8c962d8fa5c34b4a3400028cad984...
>
> he suffixed his additions with "bd", so he broke the numbering, but when one
> looks at the commit,
> one can mentally grasp what happened.
>
> I really hate to ask you this, but could you redo your commit by not
> changing other lines ?
> (You can use whatever suffix, like Antti)
> ...It really hurts me to ask you this believe me, but to review the commit
> will be as hard as
> to rewrite it and I would hate to rewrite it, since as your test case
> confirms, it looks like it's
> doing the right thing...
>
> I hope you will undersand ;-)
>
> Thanks !
>
Reply all
Reply to author
Forward
0 new messages