#1721: fetch issue comments count once per PR instead of once per review#1844
Open
morphqdd wants to merge 2 commits into
Open
#1721: fetch issue comments count once per PR instead of once per review#1844morphqdd wants to merge 2 commits into
morphqdd wants to merge 2 commits into
Conversation
Contributor
Author
|
@yegor256, please take a look when you have a moment. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1721.
judges/code-was-reviewed/code-was-reviewed.rbfetchedFbe.octo.issue_comments(repo, f.issue).countinside thereviews.eachloop, once per review — but the result doesn't depend on the review at all (it's a property of the PR, not the individual review). For a PR with N new reviews in one judge cycle, this refetched the exact same issue-comments count N times instead of once, on top of the necessarily-per-reviewpull_request_review_commentscall.Fix: hoisted the fetch into a memoized
countlocal (count ||= ...) computed once per PR, reused for every review'sn.commentsassignment.Fbe.if_absent'snext if n.nil?guard still gates whether the fetch happens at all (already-recorded reviews skip it, same as before) — only the redundant repeat for multiple new reviews on the same PR is eliminated.Added
test_fetches_issue_comments_once_for_multiple_reviews, stubbing a PR with 2 new reviews and asserting theissues/60/commentsendpoint is hit exactly once (assert_requested(..., times: 1)) instead of twice.Note: I wasn't able to run the test suite locally due to an unrelated native-gem/Ruby-version mismatch in my environment (
opensslgem compiled against a stalelibruby). I verified the change viaruby -c(syntax OK) andrubocop(no new violations vs. the pre-existing baseline — confirmed by diffing rubocop output before/after on the unmodified file) and by tracing through the logic by hand.