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

Inflection for AutoPrefetchMixin #990

Closed
wants to merge 2 commits into from
Closed

Inflection for AutoPrefetchMixin #990

wants to merge 2 commits into from

Conversation

bodevone
Copy link

@bodevone bodevone commented Oct 5, 2021

Fixes #871

Description of the Change

Perform inflection for included_resources in AutoPrefetchMixin

Checklist

  • PR only contains one change (considered splitting up PR)
  • unit-test added
  • documentation updated
  • CHANGELOG.md updated (only for user relevant changes)
  • author name in AUTHORS

@codecov
Copy link

codecov bot commented Oct 5, 2021 •
edited

Codecov Report

Merging #990 (5ecbfa7) into master (4f596d7) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #990   +/-   ##
=======================================
  Coverage   96.82%   96.82%           
=======================================
  Files          65       65           
  Lines        3906     3908    +2     
=======================================
+ Hits         3782     3784    +2     
  Misses        124      124
Impacted Files Coverage Δ
example/views.py 94.26% <0.00%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f596d7...5ecbfa7. Read the comment docs.

@sliverc sliverc self-requested a review October 6, 2021 11:28
sliverc
sliverc requested changes Oct 6, 2021
View reviewed changes
Copy link
Member

@sliverc sliverc left a comment

Choose a reason for hiding this comment

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

Thanks for working on this. Could you add a test? I think test_performance.py would be the best spot. A test could count the number of queries and if a include is not underscored a higher number of queries would be executed as auto prefetching does not get performed.

Also could you add your name to the AUTHORS file? Thanks.

@sliverc sliverc mentioned this pull request Oct 11, 2021
Open
@sliverc
Copy link
Member

sliverc commented Nov 23, 2021

I am closing this PR as it seems you won't find time to work on this any time soon. This way it also shows to other potential contributors that they can work on this issue.

If you still want to work on this PR though please comment and I am very happy to open it again.

@sliverc sliverc closed this Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sliverc sliverc sliverc requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

inflection is not performed by AutoPrefetchMixin for included resources
2 participants
@bodevone @sliverc