littleredbutton / bigbluebutton-api-php

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

Improved code quality #188

Open FelixJacobi opened 4 months ago

FelixJacobi commented 4 months ago

@SamuelWei Interested in your thoughts ;) .

fix #177 fix #162

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.11%. Comparing base (40494d0) to head (a623baf).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## 6.0 #188 +/- ## ============================================ - Coverage 99.13% 99.11% -0.03% + Complexity 440 439 -1 ============================================ Files 52 52 Lines 1037 1013 -24 ============================================ - Hits 1028 1004 -24 Misses 9 9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

FelixJacobi commented 4 months ago
SamuelWei commented 4 months ago

I think we should move all enums to the same folder/namespace.

However this is a BC. I suggest

  1. Copy the current enum classes to the enum folder and refactor using the logic used in 5.x
  2. Replacing the consts in the current namespace using aliases and deprecate all
  3. Refactor to native enums in 6.0 and remove old classes
FelixJacobi commented 4 months ago

I think we should move all enums to the same folder/namespace.

However this is a BC. I suggest

1. Copy the current enum classes to the enum folder and refactor using the logic used in 5.x

2. Replacing the consts in the current namespace using aliases and deprecate all

3. Refactor to native enums in 6.0 and remove old classes

I think, since adding types and enums to the whole code base is already a change with big impact, I would schedule code restructuring (e.g. where to place classes) for 7.0.

From consumer perspective, the good thing for now, that the baked enums on using works like the constants and mabe classes before, you don't need to change your code when upgrading. And remember: 6.0 cannot trigger new deprecations ;) (ok, except for the one in the BigBlueButton constructor, which was needed to catch typing issues when the class is initialized without environment).

FelixJacobi commented 4 months ago

To add enums here is practical, I think, we cannot introduce and use them in 5.x, since that one must still support PHP 7.4. In 5.x and 6.1, it also would mean being a breaking change.

In theory, it could introduce a few breaking changes, therefor it fits perfectly into a major release.

SamuelWei commented 3 months ago

To add enums here is practical, I think, we cannot introduce and use them in 5.x, since that one must still support PHP 7.4. In 5.x and 6.1, it also would mean being a breaking change.

In theory, it could introduce a few breaking changes, therefor it fits perfectly into a major release.

I opened #197 to show my proposed solution to move all enums to a new namespace first, allowing us to convert them to native enums later