Allow PRs to delete files, fix error passing

[jakebailey]

An effect of #761 was to make PRs fail if they ever deleted a file, no matter where. This means PRs like my PR that deletes scripts fails because deleting files is now banned, which doesn't feel right.

Also, fix a case where errors were not propagated upward, which was the root cause of the tooling hiding the error for the script PR. I think we may want to consider using a real generic result type; working with these error arrays and ensuring they're nonempty is getting a little fickle...

[sandersn]

I'm not sure the first test change is correct.

[sandersn]

I can't figure out why getNotNeededPackages was complaining about added files in the first place. Maybe it was left over from when gitDiff also processed added files?

[jakebailey]

It still does here: https://github.com/microsoft/DefinitelyTyped-tools/pull/787/files#diff-851343706958307266e65b65ada1241e0e87962c124efbd7a501d219d724a534R67

Needed to make sure we attempt to run tests on dirs that look like they're supposed to be packages. Otherwise CI passes for old style without package.json (the original change was fixing that).

[jakebailey]

But, the test doesn't seem to show that we are adding a package here for oops, so something may still be wrong...

[sandersn]

It may not matter since almost all additions are going to be (1) a single file to an existing types directory (2) multiple files attempting to add a new types directory. Mostly what we want to catch are incomplete cases of (2), and I don't think that's happening here in the first place.

[sandersn]

Actually, no, I can't find any other tests for incomplete packages. I don't even know anymore.

[jakebailey]

So generally I think that this part of the infra may be overkill:

• If someone deletes a package, the tooling needs to check the git diff to inform which packages we run as pnpm won't see the deletion.
• If someone modifies another package, it'll trigger owners and the "modifies multiple packages" check.
• If someone touches any other file, then it's an infra change and gets maintainer review.

I think this PR improves things, though doesn't remove the complexity, so it's hard to say if I've really fixed it.

[sandersn]

I think your summary is correct.
Especially as we switch toward naming .d.ts files things besides index.d.ts, deletes and moves are going to get a lot more common.