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
156 stars 70 forks source link

support for running on push in branches #154

Open cainlevy opened 3 months ago

cainlevy commented 3 months ago

Setup

I want to run CI on every push to my feature branch.

on:
  push:
    branches-ignore:
      - 'main'

Am I the first person to try this? It's strange to me that it doesn't work.

Expected

I would expect nx-set-shas to execute the git merge-base strategy.

Actual

nx-set-shas chooses the "last successful workflow" strategy for setting a BASE. Then it fails to find one and defaults to HEAD~1, with inconsistent outcomes for the robustness of the work on my branch.

Ideas

cainlevy commented 3 months ago

Workaround

In my use case, it appears that nx-set-shas can be replaced with simply:

- name: set HEAD and BASE for nx affected
  run: |
    echo "NX_HEAD=$(git rev-parse HEAD)" >> "$GITHUB_ENV"
    echo "NX_BASE=$(git merge-base origin/main HEAD)" >> "$GITHUB_ENV"
mandarini commented 3 months ago

Hi there @cainlevy ! Thank you for the feature request! We added the fallback SHA option in case a last successful workflow is not found. Would this address your issue? If not, do you want to submit a PR with the suggested implementation?

Thank you for your patience!

cainlevy commented 2 months ago

Adding a fallback SHA does not address this need. My problem is that nx-set-shas chooses the wrong strategy for running tests on my branch. It chooses the strategy intended for the main branch (deploys) rather than the strategy intended for feature development (pull requests).

I believe the problem is that nx-set-shas depends entirely on eventName to pick its strategy. This approach fails for events like push that can happen in multiple contexts.

I don't currently feel comfortable submitting an implementation change because it seems like a very delicate thing to change. This appears to be a deep assumption in the package.