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.

Sign up for GitHub

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

Jump to bottom

chore: code cleanup for php 8.1 #3

Merged
merged 3 commits into from
Nov 23, 2023
Merged

chore: code cleanup for php 8.1 #3

merged 3 commits into from
Nov 23, 2023

Conversation

Ruesa18
Copy link

@Ruesa18 Ruesa18 commented Nov 16, 2023

If those were fixed, we could raise the phpstan level to 5 with this PR:

 ------ -------------------------------------------------------------------------------------------------------------- 
  Line   src/Populator/OneFieldPopulator.php                                                                           
 ------ -------------------------------------------------------------------------------------------------------------- 
  54     Call to an undefined method Doctrine\ORM\EntityRepository<araise\SearchBundle\Entity\Index>::findExisting().  
 ------ -------------------------------------------------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------------------------------------------------- 
  Line   src/Populator/StandardPopulator.php                                                                           
 ------ -------------------------------------------------------------------------------------------------------------- 
  58     Call to an undefined method Doctrine\ORM\EntityRepository<araise\SearchBundle\Entity\Index>::findExisting().  
 ------ --------------------------------------------------------------------------------------------------------------

But I need someone else to take a look at this. I don't really get where the problem is.
We should also test this cleanup in a project to see if it holds up.
The Unit Tests pass, but if that is enough - I'm not sure... 🤔

tuxes3
tuxes3 approved these changes Nov 16, 2023
View reviewed changes
Copy link
Contributor

@tuxes3 tuxes3 left a comment

Choose a reason for hiding this comment

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

lgtm

@tuxes3
Copy link
Contributor

tuxes3 commented Nov 16, 2023

@Ruesa18 I implemented the phpstan level 5. Please test this branch locally in a application, then it can be merged!

@tuxes3
Copy link
Contributor

tuxes3 commented Nov 16, 2023

Maybe ask @rugbymauri for another review as he has developed most part of this bundle

@Ruesa18
Copy link
Author

Ruesa18 commented Nov 23, 2023

@Ruesa18 I implemented the phpstan level 5. Please test this branch locally in a application, then it can be merged!

Hey @tuxes3
I tested this in one of our demo applications and the bundle still worked, as far as I can tell. Nonetheless I would like for @rugbymauri to review the changes as well 🙂

rugbymauri
rugbymauri reviewed Nov 23, 2023
View reviewed changes
src/Entity/PreSearchInterface.php Outdated Show resolved Hide resolved
src/Manager/IndexManager.php Outdated Show resolved Hide resolved
src/Manager/IndexManager.php Outdated Show resolved Hide resolved
@tuxes3 tuxes3 marked this pull request as ready for review November 23, 2023 12:05
@tuxes3 tuxes3 merged commit 0022202 into develop Nov 23, 2023
1 check passed
@tuxes3 tuxes3 deleted the chore/cleanup branch November 23, 2023 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tuxes3 tuxes3 tuxes3 approved these changes

@rugbymauri rugbymauri rugbymauri approved these changes

Assignees

@tuxes3 tuxes3

@rugbymauri rugbymauri

@Ruesa18 Ruesa18

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Ruesa18 @tuxes3 @rugbymauri