-
Notifications
You must be signed in to change notification settings - Fork 76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
General Refactoring Feedback #124
Comments
If you see doing a refactor will yield great benefits, then I say go for it. I can make myself available for some review. I'll see if our resident front-end JS person would be interested in getting involved as well. |
No specific opposition here, I can take a look at any PRs. May be a little slow, but I'll get to them. If you go the route of adding a functional version on top of any refactoring for the current api - should it be released under a separate package though like |
Thanks for responding guys. I actually didn't get any notification from GitHub. Oops! I may take a look at this again but now I'm out of the country for a while and currently only have internet thru my phone. Re a new repo... I'll let you decide if you think it qualifies. I would think a new major version is acceptable since i don't think it would be vastly different. But whatever works. |
For no particular reason (curiosity? practice? boredom?) I was taking a pass at the repo to see how it's working. I found a few tiny details that I'm submitting as PRs.
I have been making a pass on the isChar... functions to use a more DRY approach, for example, they mostly use a new function that takes
ranges
andchar
making it possible to write most of them in pointfree style. I think it could cut down on complexity and could make some tests redundant. They're probably ever-so-slightly more optimized too.I would be interested in maybe doing a functional version of the library for a v5.0 that has autocurried functions and data-last parameter order.
I'm curious if anyone is out there who might review any of these PRs or if any of this is in any way useful to anyone?
^^ @vietqhoang @DJTB
The text was updated successfully, but these errors were encountered: