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

Fix up PHPStan errors. #1955

Merged
merged 14 commits into from
Sep 4, 2023
Merged

Conversation

michbeck
Copy link
Contributor

Road to level 8

@dereuromark
Copy link
Contributor

The other changes look good to me.

@codecov-commenter
Copy link

codecov-commenter commented Jan 27, 2023

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 79.48718% with 8 lines in your changes missing coverage. Please review.

Project coverage is 89.27%. Comparing base (011aaa7) to head (e6f2a49).
Report is 61 commits behind head on master.

Files with missing lines Patch % Lines
...nable/VersionableBehaviorObjectBuilderModifier.php 0.00% 6 Missing ⚠️
src/Propel/Generator/Manager/ModelManager.php 75.00% 1 Missing ⚠️
...e/ActiveQuery/Criterion/AbstractModelCriterion.php 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1955      +/-   ##
============================================
+ Coverage     88.56%   89.27%   +0.71%     
+ Complexity     8051     8049       -2     
============================================
  Files           243      232      -11     
  Lines         24590    24519      -71     
============================================
+ Hits          21777    21889     +112     
+ Misses         2813     2630     -183     
Flag Coverage Δ
5-max 89.27% <79.48%> (+0.71%) ⬆️
7.4 89.27% <79.48%> (+0.71%) ⬆️
agnostic 67.47% <58.97%> (+0.17%) ⬆️
mysql 69.81% <51.28%> (+0.72%) ⬆️
pgsql 69.83% <56.41%> (+0.72%) ⬆️
sqlite 67.80% <51.28%> (+0.72%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Contributor

@dereuromark dereuromark left a comment

Choose a reason for hiding this comment

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

Apart from the mixed annotations, this looks great.
Really awesome improvements going forward! Thx :)

@michbeck
Copy link
Contributor Author

@mringler @dereuromark can we move forward here?

Copy link
Contributor

@mringler mringler left a comment

Choose a reason for hiding this comment

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

Oh, sorry, thank you for the reminder!

@mringler
Copy link
Contributor

@michbeck It looks like @dereuromark has left the project, and it looks like there is no successor at Spryker at the moment. I have contacted the owner, and he told me that the project will keep being maintained by Spryker, but I haven't heard back if/when there will be someone who actively maintains Propel2, i.e. merges this PRs.
For what it's worth, I am very sorry for the situation! I really appreciate the work you put into this in the previous PRs, and I would have really liked to see this merged, preferably in acceptable time.
I guess when/if someone takes over, contributors like you and I can decide if they want to come back.
Best wishes

@floriankraemer
Copy link

floriankraemer commented Aug 31, 2023

@mringler thank you for the reminder and your patience! ❤️ I really hope you'll keep contributing after this, I'm really sorry that you had to wait, I know from personal experience that it is frustrating when your contributions won't move forward. 😞

I'll take this over temporarely until I figure out who will be our new internal owner. Your PR looks good to me! 💪 There is just one comment from Mark, to which I agree, that needs to be fixed but we could do that as well.

@michbeck can you maybe take care of it tomorrow? I'll approve the merge as soon as this has been fixed by someone and I've got the required permissions for this repository.

@floriankraemer
Copy link

@gechetspr this is ready for merge now.

@gechetspr gechetspr merged commit 4f36eea into propelorm:master Sep 4, 2023
37 checks passed
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.

7 participants