Skip to content

Conversation

@staabm
Copy link
Contributor

@staabm staabm commented Nov 10, 2025

analog #1105 but for regular line coverage data

@codecov
Copy link

codecov bot commented Nov 10, 2025

Codecov Report

❌ Patch coverage is 80.63725% with 79 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.41%. Comparing base (c1decf1) to head (acb2ed4).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/Node/File.php 66.88% 50 Missing ⚠️
src/Data/ProcessedTraitType.php 0.00% 15 Missing ⚠️
src/Report/Xml/Facade.php 78.57% 6 Missing ⚠️
src/Report/Html/Renderer/Dashboard.php 92.30% 3 Missing ⚠️
src/Node/AbstractNode.php 0.00% 2 Missing ⚠️
src/Report/Clover.php 94.11% 1 Missing ⚠️
src/Report/Crap4j.php 90.90% 1 Missing ⚠️
src/Report/OpenClover.php 94.44% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1109      +/-   ##
============================================
- Coverage     88.56%   88.41%   -0.15%     
- Complexity     1382     1387       +5     
============================================
  Files            98      102       +4     
  Lines          4651     4712      +61     
============================================
+ Hits           4119     4166      +47     
- Misses          532      546      +14     

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@staabm
Copy link
Contributor Author

staabm commented Nov 10, 2025

very interessting.. this PR does not yet yield a measurable performance improvement in the cases I tested. neither perf nor memory wise.

still its a improvement regarding maintainability and readability.

@sebastianbergmann
Copy link
Owner

Why is this still a draft? Thanks.

@staabm
Copy link
Contributor Author

staabm commented Nov 11, 2025

because it did not yield the expected perf results. if you are fine with the change we can still go ahead

@staabm staabm marked this pull request as ready for review November 11, 2025 06:41
@staabm
Copy link
Contributor Author

staabm commented Nov 15, 2025

I think this is good to go.

final class ProcessedClassType
{
public function __construct(
public readonly string $className,

Choose a reason for hiding this comment

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

Please do not introduce property promotion, thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here we go.

remaining CI error is related to a deprecated php-cs-fixer setting, not the PR itself

Choose a reason for hiding this comment

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

Thank you! I have updated the configuration for PHP-CS-Fixer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rebased

@staabm staabm force-pushed the lineobj branch 4 times, most recently from 65603c9 to c400997 Compare November 19, 2025 07:38
@staabm
Copy link
Contributor Author

staabm commented Nov 19, 2025

Btw: motivation for this PR is also more strict structures, so we can level up phpstan and prevent bugs in the future

@staabm
Copy link
Contributor Author

staabm commented Nov 20, 2025

would be great to get this merged, as it likely conflicts with other de-DOM-mification work along the way

@sebastianbergmann sebastianbergmann merged commit 45dff46 into sebastianbergmann:main Nov 20, 2025
14 of 16 checks passed
@staabm staabm deleted the lineobj branch November 20, 2025 07:26
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.

2 participants