cljs-time icon indicating copy to clipboard operation
cljs-time copied to clipboard

Use upper-bound correctly in parse-number fn

Open dvdreddy opened this issue 7 years ago • 3 comments
trafficstars

  • Lower / Upper bound was not being correctly enforced when trying to read a number
  • Also force the default radix to be 10 to avoid irregular behavior of the parse fn

Fixes #124

dvdreddy avatar Feb 09 '18 08:02 dvdreddy

Would it be good to add some tests exercising this change?

danielcompton avatar Feb 09 '18 08:02 danielcompton

Just for the parse-number I can surely add,

But for overall parsing I tried to come up with a case which fails the formatting for date-time, I came up short mainly because the current code works fine due to the separators.

dvdreddy avatar Feb 09 '18 08:02 dvdreddy

If you can write some tests for parse-number that fail under the old implementation, that would be great.

But for overall parsing I tried to come up with a case which fails the formatting for date-time, I came up short mainly because the current code works fine due to the separators.

Can you explain this a little more? I think I might know what you mean, but it'd be good to have an example to make sure we're on the same page.

danielcompton avatar Dec 10 '18 10:12 danielcompton