| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
Thanks for the pull request @marshall007. I think this is your first C++ PR? :-) I'll take a look at it later. |
Sorry, something went wrong.
| while (!is_exhausted()) { | ||
| double next = safe_to_double(start++); | ||
| double next = safe_to_double(start); | ||
| start+=step; |
There was a problem hiding this comment.
Per our usual style, there should be spaces around the operator: start += step;
Sorry, something went wrong.
|
I added a few comments, but looks good mostly. Are you planning to add support in the other drivers as well? We can take over that part if you prefer. |
Sorry, something went wrong.
|
Just saw that there was some discussion in #3148 on whether this option should be called stride or step. I personally like step better. I'll ask a few people around here for what they think on Monday. |
Sorry, something went wrong.
@danielmewes yep! :D I've made updates based on your comments and refactored all those separate optarg checks in the JS driver into util.isPlainObject(obj). I replied to your comment regarding the behavior when step == 0. Just let me know if you think we should support that.
Would be great if someone else could take over, thanks! |
Sorry, something went wrong.
|
Thanks for updating the PR and for the isPlainObject refactor. Let's keep disallowing step == 0. It avoids potentially unexpected behavior and allowing something later is always easier than the other way around (with respect to backwards compatibility). The only remaining question is the name of the option. I'll talk to some people to figure this out and let you know later today. |
Sorry, something went wrong.
|
@marshall007 After talking to some people, we settled on step. The server side of this looks good to me. I haven't reviewed the JavaScript driver changes yet. |
Sorry, something went wrong.
|
@danielmewes sweet! I went ahead and updated the Python driver too. |
Sorry, something went wrong.
|
What's left is to do the Ruby and Java drivers. Working on that now... Edit: Actually, I don't think people would like more new stuff getting pushed into 2.4. Making this on milestone 2.5. |
Sorry, something went wrong.
|
Pointing this at v2.4.x in preparation for making that branch the main branch of the repo. I intend to look at this post-2.4.2 release. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Fixes #3148. Thus far I've only updated the argument parsing logic for r.range in the JS driver.