search

Mic92 / nixpkgs-review

Review pull-requests on https://github.com/NixOS/nixpkgs
MIT License
379 stars 63 forks source link

would using pull/x/merge allow the usage of a shallow clone? #393

Open lolbinarycat opened 6 months ago

lolbinarycat commented 6 months ago

i thought i made an issue like this before, but now i can't find it.

iirc, before, the problem was that being able to find the merge point required having enough git history that you could find the commit the PR was targeted at.

what if we skip the merge step entirely and make github do it for us?

Mic92 commented 6 months ago

Does this significantly save storage over long time i.e. if you have to run this on many pull requests? One problem I see with buildbot is that these pull/x/merge can be sometimes not found shortly after the pull request creation because github needs to compute those. So there would be some sort of fallback needed in case github is slow again.

lolbinarycat commented 6 months ago

Does this significantly save storage over long time i.e. if you have to run this on many pull requests?

well, the merged state should be semi-transient anyways (i believe git gc will remove it?), so i would think so?

i think storing the entirety of nixpkgs commit history should be large enough that it dwarfs any other overhead.

One problem I see with buildbot is that these pull/x/merge can be sometimes not found shortly after the pull request creation because github needs to compute those. So there would be some sort of fallback needed in case github is slow again.

true, although the overhead could be just relying on the current behavior.

also, most pull requests on nixpkgs sit for a while before being reviewed, so that shouldn't be a huge issue.