Skip to content

Add support for blameable on remove#2929

Open
oojacoboo wants to merge 16 commits intodoctrine-extensions:mainfrom
rentpost:blameable-on-remove
Open

Add support for blameable on remove#2929
oojacoboo wants to merge 16 commits intodoctrine-extensions:mainfrom
rentpost:blameable-on-remove

Conversation

@oojacoboo
Copy link
Copy Markdown
Contributor

This PR adds support for blameable on remove/deletion of an entity. This can be very useful, when used in conjunction with soft-delete. It obviously is useless without soft-delete.

Docs have been updated a tests added.

Comment thread src/AbstractTrackingListener.php Outdated
Comment thread doc/blameable.md Outdated
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.80%. Comparing base (eef8a13) to head (77c7354).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2929      +/-   ##
==========================================
+ Coverage   78.75%   78.80%   +0.04%     
==========================================
  Files         169      169              
  Lines        8695     8715      +20     
==========================================
+ Hits         6848     6868      +20     
  Misses       1847     1847              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@oojacoboo
Copy link
Copy Markdown
Contributor Author

Couple things here:

  • I don't understand why PHP 7.x is being supported still. Why can't people that need 7.4 support use an older version of this lib? How long do you guys intend on trying to continue to support it? It makes it difficult trying to properly type this lib when supporting such a old version. PHP isn't even supporting PHP 8.0 anymore. Annotations (only attributes supported) should be entirely dropped from this lib IMO, for all future versions.
  • What's going on with the PHPStan errors? I didn't make changes to these files.

@phansys
Copy link
Copy Markdown
Collaborator

phansys commented Mar 24, 2025

What's going on with the PHPStan errors? I didn't make changes to these files.

You are removing @phpstan-template annotations, so errors like this are expected:

Error: Generic type Gedmo\Mapping\MappedEventSubscriber<array, Gedmo\Mapping\Event\AdapterInterface> in PHPDoc tag @extends specifies 2 template types, but class Gedmo\Mapping\MappedEventSubscriber supports only 1: TConfig

@oojacoboo
Copy link
Copy Markdown
Contributor Author

oojacoboo commented Mar 26, 2025

@phansys I've resolved the PHPStan issues. I'm a bit confused with what's going on with PHPStan on this lib, though. There is a phpstan-baseline.neon file that's over 1200 lines of ignores. Also, does PHPStan not read the default phpdoc annotations now (@param, @return, etc)?

Also, IMO, it'd be a good idea to drop support for some of these unsupported versions of PHP. It's holding back the lib and making updates and codestyle much more difficult.

Please let me know if anything else is needed here - should be ready.

@oojacoboo
Copy link
Copy Markdown
Contributor Author

@mbabker @phansys Can we get this merged?

@oojacoboo
Copy link
Copy Markdown
Contributor Author

@phansys I've merged in the latest changes. There weren't any conflicts though, so it wasn't actually necessary. But that's been done. I'm unsure about this failing test though. That seems to be preexisting, unless I'm misunderstanding something.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants