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

BC Break on 2.2.0 #887

Closed
volkyeth opened this issue Sep 24, 2019 · 5 comments
Closed

BC Break on 2.2.0 #887

volkyeth opened this issue Sep 24, 2019 · 5 comments

Comments

@volkyeth
Copy link
Contributor

#852 introduced some BC breaks on src/State/Scope.php by removing the public methods:

  • getTags
  • getExtra
  • getUser
  • getFingerprint
  • getLevel
  • getBreadcrumbs

this change broke the upstream as predicted on getsentry/sentry-symfony#239

Don't know what's the versioning schema over here, but maybe you guys could revert it and schedule the deprecation for 3.0.0, as most upstreams probably use semver

@ste93cry
Copy link
Collaborator

All those methods were marked as @internal and their only purpose was to make unit tests work. When we rewrote the mentioned tests to not use them they became useless. As the documentation about the tag says, code marked as internal should not be used by the public and if you do you do it at your own risk. There is indeed a technical BC here, but not in the way you are supposed to be interested in. Summarizing: everything marked as @internal does not fall into the BC promise. We usually follow the BC promise of Symfony as it's pretty clear and explanatory of what can and can not be done (of course some things may not apply to our project). I will quote some documentation excerpts here for future reference:

The @internal tag can be used as counterpart of the @api tag, indicating that the associated Structural Elements are used purely for the internal workings of this piece of software.

Experimental Features and code marked with the @internal tags are excluded from our Backward Compatibility promise.

@volkyeth
Copy link
Contributor Author

My bad, I hadn't noticed the @internal on those diffs. 😅
Sorry about the bother, I'll follow up on sentry-symfony

@Jean85
Copy link
Collaborator

Jean85 commented Sep 24, 2019

sentry-symfony is ready to go, we will tag a new version that will include getsentry/sentry-symfony#239 ASAP. Right now the last tagged version just triggers a few deprecations; those removed methods where used just in tests there too.

@volkyeth
Copy link
Contributor Author

TYSM @Jean85

@ste93cry
Copy link
Collaborator

A good idea may be to update the README to specify the fact that we strive to follow Semver and also that we try to follow as close as possible the BC Promise document from Symfony. Any contribution is more than welcome on this 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants