GitoxideLabs / gitoxide

An idiomatic, lean, fast & safe pure Rust implementation of Git
Apache License 2.0
8.91k stars 303 forks source link

"Tests pass" blocks PRs that don't need tests #1608

Closed EliahKagan closed 2 weeks ago

EliahKagan commented 2 weeks ago

Current behavior 😯

Tests run on pull requests when changes are made to files in:

https://github.com/Byron/gitoxide/blob/612896d7ec15c153d3d48012c75aaf85d10a5abe/.github/workflows/ci.yml#L28-L37

Otherwise they are intentionally skipped as unnecessary.

That applies to the ci.yml workflow as a whole. The "Tests pass" check, which is defined in that workflow, as well as the actual test jobs it depends on, do not run on pull requests that don't make any of those modifications. An example of such a pull request is #1607.

Expected behavior 🤔

If there is a way to do it that works well, only relevant checks should be required for pull requests.

A possible alternative is to keep things as they are and handle merging of such pull requests manually. Most pull requests do run tests, so auto-merging is still very useful even if this is not fixed. But I suspect there may be a reasonable way to achieve the effect of only requiring checks when they are relevant.

Another consideration, though I'm not sure if it matters, is that PRs are sometimes opened with minimal initial contents, sometimes even empty. Such a PR intentionally does not run unnecessary tests when opened. There might be a disadvantage to such a PR being auto-mergeable, even if it has a merging review from a user whose reviews would otherwise permit that.

Git behavior

As far as I can quickly tell, neither git/git nor git-for-windows/git use the GitHub auto-merging feature. (git/git is also a mirror, and its PRs work via GitGitGadget, so it wouldn't necessarily be comparable in this area.)

Steps to reproduce 🕹

There are two ways to verify this: