search

rust-lang / triagebot

Automation/tooling for Rust spaces
https://triage.rust-lang.org
Apache License 2.0
172 stars 74 forks source link

Only fetch PR diff once. #1749

Closed ehuss closed 10 months ago

ehuss commented 10 months ago

This adds an optimization and simplification for getting the diff of a PR. Previously, triagebot would fetch the /compare endpoint three times for every PR (for auto-assignment, auto-labels, and mentions). This changes it so that it only fetches it once, and caches the result in the PullRequestDetails.

This also changes it so that the diff gets parsed early on, instead of having every caller parse it. This should help make it a little more structured. Thus, diff() returns a Vec<FileDiff>, where each element is a single file.

A walkthrough to help reviewing this PR:

  1. Added a FileDiff type to represent a diff to a single file.
  2. PullRequestDetails adds a OnceCell to cache the PR diff.
  3. Issue.diff() now returns a parsed result (instead of just a string), and caches the result.
  4. files_changed was changed to parse_diff to return a structured format.
  5. All the callers of Issue.diff() were updated to use the new format.
  6. The assign.rs input handler was changed to fetch the diff in handle_input instead of parse_input, due to not being able to pass a reference (and it wasn't important which function it was called in).
  7. Auto-assignment was doing its own diff parsing which is no longer necessary.