Skip to content
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

Update to PHP 8 and PHPUnit 10 #122

Closed
wants to merge 5 commits into from
Closed

Update to PHP 8 and PHPUnit 10 #122

wants to merge 5 commits into from

Conversation

gbirke
Copy link
Contributor

@gbirke gbirke commented Oct 30, 2023

This fixes #121 , #120

gbirke and others added 5 commits October 25, 2023 12:04
The library was activating `remoteConfig` when it was set to `false`,
giving the user no option to disable it.
Update to 8.0 to be able to run with PHP 8 and installed with composer
on PHP 8, convert tests from old PHPUnit format to PHPUnit 8. Choosing
PHPUnit 8 as an intermediate step that has enough changes, but also has
some deprecations that will be removed in future PHP versions.

The only change to the library is to add a property that formerly was
implicit and to move the constant AIRBRAKE_NOTIFIER_VERSION from
Notifier.php to a separate file that will be autoloaded. This removes
the dependency on "accidentally" autoloading the constant when loading
the Notifier class. In the future we might think about making it a
class constant.

Update php_codesniffer to a version that's compatible with PHP 8
Migrate the configuration file
Make data providers static
Replace renamed assertion
@thompiler
Copy link
Member

Hi there! Thanks for the PR! We will take a look.

@@ -12,8 +12,6 @@
define('ERR_IP_RATE_LIMITED', 'phpbrake: IP is rate limited');
define('ERR_NOTIFICATIONS_DISABLED', 'phpbrake: error notifications are disabled');

const AIRBRAKE_NOTIFIER_VERSION = '0.8.0';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this constant removed?

When I run the unit tests, I see errors mentioning the removed constant. Example:

15) Warning
The data provider specified for Airbrake\Tests\NotifierTest::testPostsToURL is invalid.
Undefined constant "Airbrake\AIRBRAKE_NOTIFIER_VERSION"

@mmcdaris mmcdaris mentioned this pull request Dec 6, 2023
@mmcdaris
Copy link
Member

mmcdaris commented Dec 6, 2023

Thanks for your contribution @gbirke, I've merged in the alternate PR that included recent CI and tests via docker updates. Closing this one out in favor of #125

@mmcdaris mmcdaris closed this Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PHP 8.2: Dynamic Properties are deprecated
4 participants