Skip to content

Do not create ChangesetSpecs on server with empty diff

Warren Gifford requested to merge mrn/no-empty-diffs into main

Created by: mrnugget

I ran into this yesterday with Chris: we ran a command across multiple repositories and in one repository it didn't produce a diff.

That's totally to be expected, since you don't know which command will produce which diff in which repository.

The problem is that src-cli then sends up the changeset spec, even though the diff is empty and we then try to apply the diff in gitserver, which is where it fails (bonus: our error message looked really good!)

This commit here fixes it by not adding a spec with an empty diff to the "list of completed specs".

I've looked at a few places where we could add this check (for example: right before we make the GraphQL request we could do an early exit, or before we add it to the cache, ...) and I'm still not 100% whether that's the best place.

But I think the requirements are these:

  • We should not send up empty diffs (because it looks weird in the UI if we say "we will create this changeset", even though it has no diff).
  • We should still cache the result, even though the diff is empty.
  • We should be able to report progress when executing it (we did do something, but we just didn't produce a diff)
  • We shouldn't say "sending up 1 changeset spec" if we don't do that

That's why I added it where it is, which also required to fix the progress bar for the case where "bar.Max == 0" so we don't run into a panic.

I think this is the least invasive option of all the ones I considered (I could also, for example, blast a few if len(specs) > 0 around the codebase...).

Let me know what you think, especially you @LawnGnome!

Merge request reports