search

littleredbutton / bigbluebutton-api-php

Unofficial (but better) PHP API for @BigBlueButton
GNU Lesser General Public License v3.0
25 stars 12 forks source link

Added parameter typehints where possible #82

Closed FelixJacobi closed 2 years ago

FelixJacobi commented 3 years ago

This adds parameter type hints to all existing code where possible (i.e. the code base was clear enough about types). I also removed docblock entries everywhere where their give no further information besides the type hints. Also some classes were marked as @final and @abstract which should become final or abstract starting from the next release (4.1).

This is possible as this PHP code is legal:

class Base
{
    public function foo(string $bar);
}

class Child extends Base
{
   public function foo($bar);
}

If you e.g. extended the classes here without parameter typehints this will continue to work.

This does not add return type hints yet as there behaving inverted to the parameter type hints (if your child class does not note the ones from parent this is resulting in a fatal error). I think we should discuss to also add return types for the 4.0 release as a BC break is legal for a major release.

sualko commented 3 years ago

I think we should discuss to also add return types for the 4.0 release as a BC break is legal for a major release.

This means we delay the current release?

FelixJacobi commented 3 years ago

I think we should discuss to also add return types for the 4.0 release as a BC break is legal for a major release.

This means we delay the current release?

Yeah, you are right. I thought I would have the time to work on this the next days, but this was not the case. My proposal: We should delay return types in direction of version 5.0 and moving this PR here to 4.1 (which is possibly due to not being a breaking change) to decouple changes. I also would spend some time to improve (unit) test coverage before merging this. I would only add #78 to the window of 4.0 as it contains a bug fix for PHP 8 and then close the window for that release.

sualko commented 2 years ago

It would be nice if we could merge this sometime soon. @FelixJacobi what needs to be done until this can me merged? I would do it.

FelixJacobi commented 2 years ago

It would be nice if we could merge this sometime soon. @FelixJacobi what needs to be done until this can me merged? I would do it.

At least Coveralls must be fixed ^^ .

My plan was to write unit tests against the old code to ensure that nothing breaks here - as the code touched has a few untested paths.