flightphp / core

An extensible micro-framework for PHP
https://docs.flightphp.com
MIT License
2.61k stars 407 forks source link

added some methods to help with async frameworks #530

Closed n0nag0n closed 6 months ago

n0nag0n commented 6 months ago

I'm not opposed to that, but I feel like that needs to be addressed everywhere haha. There are some poorly worded methods and variable names that make me wonder what they are. I could do this for just this one method (and the other register method as well) but this is a much larger undertaking :laughing:

fadrian06 commented 6 months ago

oh wow, we have an issue, the suggestion was just to change the parameters of the unregister() method😵‍💫

This is because it is a new method that was not previously in the API and therefore tied to a contract.

While the other methods do have to keep their name or there will be breaking changes

in php 8 they added named arguments...if the parameter names change, the named arguments will throw an error in flight users' code

In the unregister method it is good to name the parameters better because it is a new method, the others are not new and there will already be people who use them

n0nag0n commented 6 months ago

Ahh, good catch :) I'm still stuck in PHP 7.4 land.......so......then my vote is to be consistent instead of "right" and leave it as $name because $name is already in the register function and I wouldn't want people to be confused as to why it's called $name in one but $methodName in another. That's why I changed register() as well.

cdsaenz commented 6 months ago

Ahh, good catch :) I'm still stuck in PHP 7.4 land.......so......then my vote is to be consistent instead of "right" and leave it as $name because $name is already in the register function and I wouldn't want people to be confused as to why it's called $name in one but $methodName in another. That's why I changed register() as well.

Same here for now in 7.4 which I think is the minimum sane version for PHP (and probably for some time!)