search

nrwl / nx-set-shas

:sparkles: A Github Action which sets the base and head SHAs required for `nx affected` commands in CI
https://nx.dev
MIT License
150 stars 71 forks source link

Unexpected shas for Merge Queues #140

Open Fitzbert-Fitz opened 5 months ago

Fitzbert-Fitz commented 5 months ago

Hi there, as also explained here #100 we encounter massive building overhead due to the sha calculation. What we would expect: assume you have two PRs queued in the merge queue. Tha latest main build is successful. We would assume that Queue positon '#1' would derive latest main(successful) as base sha. Position '#2' in the queue would derive position '#1' as base sha since it assumes that position '#1' will pass the queue as well. image

But what we see is that the base sha is always the git merge-base / the common ancestor. Meaning Position '1#' in the queue builds all changes since the branch was created not taking into account that builds on main might have been successful in the meantime. The same for position '#1' in the queue. See below: image

Looking at the code we do not understand why git merge-base is used here. It might be a good fall back in case you do not find successful commits on main. Maybe miss ordered inside the if else statement? https://github.com/nrwl/nx-set-shas/blob/76907e7e5d3cd17ddb5e2b123389f054bffcdd03/find-successful-workflow.ts#L42-L62

mandarini commented 4 weeks ago

Hi there @Fitzbert-Fitz ! Thanks for filing an issue! Do you think the changes suggested by @paulo9mv in the PR linked above would solve your issue? Do have a suggested implementation that would make things smoother?