Range enum #22
Range enum #22
Conversation
marcandre
force-pushed
the
range_enum
branch
from
4dd7fc5 to
b3ee75c
Compare
There was a problem hiding this comment.
Looks good to me, just have one point to clarify.
About type: {} -- yes, it was probably a false move from my side.
The sad story of this library is: the project I needed it for, is indefinitely postponed, and the library hangs in an "it is the prototype, we'll clarify a lot of things later" since then (some parts of what I've written 2 years ago surprised me myself, TBH)
I still not sure if I just want to "let the library go" (e.g. probably move it to another maintainer), as I kinda have a clear vision for a lot of things in the future, and haven't given up completely on the ideas...
That's where the weird state of things come from. I'll try to do something on it (either reschedule my activity to become a more reasonable maintainer or else).
| end | ||
|
|
||
| def validation_error(value) | ||
| return 'is not an Integer' if type.begin.is_a?(Integer) && !value.is_a?(Integer) |
There was a problem hiding this comment.
This seem surprisingly narrow. Care to explain?..
There was a problem hiding this comment.
If the range is 1..42, then only integer values would be acceptable. If the range is 1.0..42.0, then float or integers would be ok. I don't need the floating range personally, but I thought supporting it this way made sense.
There was a problem hiding this comment.
Hm ok, let's leave it this way for now. My intuition says it means we need to somehow separate "expected type" and "expected range" from range, but it is non-trivial (because naive solution would prohibit integer for float ranges, for example), so may be I am overthinking. So let's clarify this thing with validate_error in neighbour PR, and then merge.
marcandre
force-pushed
the
range_enum
branch
from
b3ee75c to
33027fc
Compare
|
I don't plan on rebasing this PR in the near future. |
|
I understand. |
This PR improves handling of
param :foo, enum: 1..1000. It also allows float enums.This builds on #21; only the last commit is new.
Note: it's not clear to me why the API allows
type: {a: 1},enum: {a: 1}andenum: 1..42but nottype: 1..42. I find this inconsistent and would suggest to either allow it, or forbidtype: {...}. This PR doesn't address this.