From 5337f26ff21367b9dd39a8a3ba39244bbbacbcc7 Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Fri, 10 Aug 2018 12:00:26 -0300 Subject: [PATCH] Update diff docs regarding note diff file usage --- doc/development/diffs.md | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/doc/development/diffs.md b/doc/development/diffs.md index 55fc16e0b33..d06339480b1 100644 --- a/doc/development/diffs.md +++ b/doc/development/diffs.md @@ -1,6 +1,6 @@ -# Working with Merge Request diffs +# Working with diffs -Currently we rely on different sources to present merge request diffs, these include: +Currently we rely on different sources to present diffs, these include: - Rugged gem - Gitaly service @@ -11,6 +11,8 @@ We're constantly moving Rugged calls to Gitaly and the progress can be followed ## Architecture overview +### Merge request diffs + When refreshing a Merge Request (pushing to a source branch, force-pushing to target branch, or if the target branch now contains any commits from the MR) we fetch the comparison information using `Gitlab::Git::Compare`, which fetches `base` and `head` data using Gitaly and diff between them through `Gitlab::Git::Diff.between` (which uses _Gitaly_ if it's enabled, otherwise _Rugged_). @@ -32,6 +34,17 @@ In order to present diffs information on the Merge Request diffs page, we: 3. If the diff file is cacheable (text-based), it's cached on Redis using `Gitlab::Diff::FileCollection::MergeRequestDiff` +### Note diffs + +When commenting on a diff (any comparison), we persist a truncated diff version +on `NoteDiffFile` (which is associated with the actual `DiffNote`). So instead +of hitting the repository every time we need the diff of the file, we: + +1. Check whether we have the `NoteDiffFile#diff` persisted and use it +2. Otherwise, if it's a current MR revision, use the persisted +`MergeRequestDiffFile#diff` +3. In the last scenario, go the the repository and fetch the diff + ## Diff limits As explained above, we limit single diff files and the size of the whole diff. There are scenarios where we collapse the diff file,