Follow-up from "Fix authoritative source of truth for diff files when loading batched diffs"

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

• Work on this issue
• Close this issue

The following discussion from !23274 (merged) should be addressed:

• @pslaughter started a discussion: (+1 comment)

  suggestion (non-blocking): This part is a bit awkward because our main motivation for calling prepareDiffData in this context is to initialize our diff object (not necessarily combine or dedupe anything). This is a smell that our prepareDiffData is not cohesive and is doing too much (i.e. merging with previous files and setting up the object given to it).

  IMHO, we could simplify things a bit if we split up prepareDiffData into

  import { flow } from 'lodash';
  
  export const prepareDiffFile = flow([
    ensureBasicDiffFileLines,
    prepareDiffFileLines,
    finalizeDiffFile,
  ]);
  
  export function prepareDiffData(files = []) {
    return files.map(prepareDiffFile);
  }
  
  export function mergeDiffFiles(newFiles, priorFiles = []) {
    return deduplicateFilesList([...priorFiles, ...newFiles]);
  }

  If you think something like this is helpful, can we handle it here or in a follow-up?