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

Possible regression in 1.24.0 release #1216

Closed
tractorcow opened this issue Nov 6, 2018 · 2 comments
Closed

Possible regression in 1.24.0 release #1216

tractorcow opened this issue Nov 6, 2018 · 2 comments

Comments

@tractorcow
Copy link

tractorcow commented Nov 6, 2018

In subclases of RavenHandler you could have a private $logLevels method in order to replicate the value in the parent class. As this is now protected, those subclasses will now all cause PHP errors on upgrade to 1.24.0.

An example of the error is:

Fatal error: Access level to PhpTek\Sentry\Monolog\Handler\SentryRavenHandler::$logLevels must be protected (as in class Monolog\Handler\RavenHandler) or weaker in /Users/damian.mooyman/Sites/testsite/vendor/phptek/sentry/src/Handler/SentryRavenHandler.php on line 23

You can fix this in the subclass by removing the private variable, but I just thought I'd ping back that this upgrade has a tiny risk of breakage.

See phptek/silverstripe-sentry#18 for reference.

I'm not sure if monolog is following semver. Apologies in advance if this isn't a valid bug, but I thought it would be better to provide feedback than not. :)

@Seldaek
Copy link
Owner

Seldaek commented Nov 6, 2018

That's "funny" because the change was done to facilitate extending the class, and it causes problems with someone extending the class..

We do follow semver, but to be honest at this point we can't possibly do any code change if we want to be 100% BC.

Sounds good that you have a fix going, I'd say move on with it and let's put this behind..

@Seldaek Seldaek closed this as completed Nov 6, 2018
@tractorcow
Copy link
Author

Thanks I appreciate the response @Seldaek :D I understand it's never really possible to be 100% backwards compatible.

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

No branches or pull requests

2 participants