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

The method allPopulate in ActiveQuery will execute populate twice during execution #289

Closed
wants to merge 54 commits into from

Conversation

niqingyang
Copy link
Contributor

Q A
Is bugfix? ✔️
New feature?
Breaks BC? ✔️/❌
Fixed issues The method allPopulate in ActiveQuery will execute populate twice during execution #286

niqingyang and others added 30 commits December 9, 2023 16:50
…y so that the IDE can automatically identify the specific class of ActiveRecord
…y so that the IDE can automatically identify the specific class of ActiveRecord
…y so that the IDE can automatically identify the specific class of ActiveRecord
…y so that the IDE can automatically identify the specific class of ActiveRecord
…y so that the IDE can automatically identify the specific class of ActiveRecord
niqingyang and others added 22 commits December 28, 2023 01:07
…y so that the IDE can automatically identify the specific class of ActiveRecord
…y so that the IDE can automatically identify the specific class of ActiveRecord
…y so that the IDE can automatically identify the specific class of ActiveRecord
…y so that the IDE can automatically identify the specific class of ActiveRecord
…y so that the IDE can automatically identify the specific class of ActiveRecord
…y so that the IDE can automatically identify the specific class of ActiveRecord
…y so that the IDE can automatically identify the specific class of ActiveRecord
…y so that the IDE can automatically identify the specific class of ActiveRecord
Co-authored-by: Sergei Tigrov <[email protected]>
Copy link

what-the-diff bot commented Jan 6, 2024

PR Summary

  • Simplification of 'allPopulate()' method in ActiveQuery.php
    The previous version of the allPopulate() method used to populate rows and carry out additional checks to ensure they weren't empty. With this update, this process got simpler as we replaced this logic with a direct call to the all() method for a more efficient execution.

Copy link

codecov bot commented Jan 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a9097f1) 87.70% compared to head (f95a71d) 87.67%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #289      +/-   ##
============================================
- Coverage     87.70%   87.67%   -0.03%     
+ Complexity      595      594       -1     
============================================
  Files             7        7              
  Lines          1326     1323       -3     
============================================
- Hits           1163     1160       -3     
  Misses          163      163              

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

Copy link
Member

@Tigrov Tigrov left a comment

Choose a reason for hiding this comment

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

Sorry for the long response. Perhaps we should remove the allPopulate() method as this is the same as all().

Related with:

@niqingyang
Copy link
Contributor Author

Sorry for the long response. Perhaps we should remove the method as this is the same as .allPopulate()``all()

Related with:

Yes, it looks better this way

@niqingyang niqingyang closed this by deleting the head repository Jan 29, 2024
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