SuperClosure v2.0-alpha1 Feedback about super_closure HOT 12 CLOSED

Probably nitpicking, but where are the constants SC_HANDLE_CLOSURE_BINDINGS and various SC_* defined? Couldn't find them...

from super_closure.

Nitpicking is allowed. 😄 First make sure you are in the multiple-parsers branch. Then look in https://github.com/jeremeamia/super_closure/blob/multiple-parsers/src/functions.php. If you are up to doing a little code review, that would be great. I released this as an alpha, because I wanted to get some more eyes on it, and I'm expecting a little more churn before I finalize everything. If you see anything in the code that looks silly to you or that you have an alternate idea for, please let me know. (I still have some work to do on docblocks though, so don't worry about those)

from super_closure.

Ok - given that, could you please namespace them? :-) I'd actually like to see a PR for the 2.x branch - that would make reviewing much easier

from super_closure.

Good call, here you go: #11

from super_closure.

@Ocramius Thanks for all of the comments. I'm also wondering what you think from a high-level. It seems you are not a fan of the serialize_closure function, but do you have any other ideas about how to make it easy to use with the configuration options? Should I change my approach completely. What about the config options and what I selected as defaults? Do you prefer faster by default or more robust by default? Anything can change at this point still.

from super_closure.

@jeremeamia I'm fine with the function, since it doesn't cause side effects.

Robust by default is also a good way to go - speed is a thing that can be tweaked later on :-)

from super_closure.

@Ocramius I agree.

from super_closure.

Can we look to support nikic/php-parser 1.0 rather than 0.9? I know it's early days for it, but I recon it would be in a later stage of development by the time this repo hit's a stable 2.0.

from super_closure.

@GrahamCampbell I'll definitely check it out once it is closer to 1.0. I need to get the test suite built up for SuperClosure again first. After that, swapping in the newer PHPParser should be fairly easy.

from super_closure.

Awesome! Thanks for all your work on this superb package.

from super_closure.

I've sent a pull (#22) to upgrade us to php parser 1.0 since the api is now stable, and a stable tag will come in 2 week's time. See: nikic/PHP-Parser#125 (comment).

from super_closure.

The master branch now contains a new iteration on what I had with this branch.

from super_closure.