-
-
Notifications
You must be signed in to change notification settings - Fork 4
Use make-prefab-struct instead of constructor for prefab structs #4
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
Conversation
I think there are tests for pconvert somewhere. Also, you might use for/list and in-vector to avoid the intermediate list. |
I just added a test. |
For that I used the same pattern as https://github.com/racket/pconvert-lib/pull/4/files#diff-cb76abd3c1f365f0647a55475b35155eL475 a few lines later, but with a direct use of recur instead of a lambda. |
What do you think? |
Looks reasonable to me. Avoiding the intermediate list would look like
|
Do you want me to change the https://github.com/racket/pconvert-lib/pull/4/files#diff-cb76abd3c1f365f0647a55475b35155eL475 a few lines later too? Come to think of it I could change https://github.com/racket/pconvert-lib/pull/4/files#diff-cb76abd3c1f365f0647a55475b35155eL163 to avoid intermediate lists as well. (Edit: and this https://github.com/racket/pconvert-lib/pull/4/files#diff-cb76abd3c1f365f0647a55475b35155eL160) (Further edit: and this https://github.com/racket/pconvert-lib/pull/4/files#diff-cb76abd3c1f365f0647a55475b35155eL281) (Further edit: and this https://github.com/racket/pconvert-lib/pull/4/files#diff-cb76abd3c1f365f0647a55475b35155eL394) |
Up to you. That could also be a separate change -- I think this is good to go as is. |
I think it should be a separate change. |
I'll make a separate pull request after. |
Are you going to merge? |
Done. |
goes with racket/pconvert#4 Closes #2.
Is |
That wouldn't shock me. Porting the code to |
closes #3