search

verilog-to-routing / vtr-verilog-to-routing

Verilog to Routing -- Open Source CAD Flow for FPGA Research
https://verilogtorouting.org
Other
979 stars 378 forks source link

Prevent running the CI if only the docs are updated #2598

Closed ueqri closed 2 weeks ago

ueqri commented 3 weeks ago

Proposed Behaviour

We should prevent the CI running (particularly, the .github/workflows/test.yml) if only the documentation files are updated.

Current Behaviour

Any push or PR that includes ONLY documentation changes (e.g., changes to the developer guide or README) will trigger all current CI workflows, which could be unnecessary and a waste of CI resources.

Possible Solution

Add a path-ignore filter to the GitHub Actions workflow triggers to filter out the docs changes, https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#example-excluding-paths

When all the path names match patterns in paths-ignore, the workflow will not run. If any path names do not match patterns in paths-ignore, even if some path names match the patterns, the workflow will run.

Specifically, we can add path-ignore filters to the push and pull_request triggers (using use push as an example below):

push:
  paths-ignore: # Prevent from running if only docs are updated
    - 'doc/**'
    - '**/*README*'
    - '**.md'
    - '**.rst'

Small limitations: in the PR scenario, the filter will consider all commits (not just a single, or the latest commit) of a PR. In this case, any commit that includes a file not matching the patterns, along with any subsequent commits, will trigger the workflow.

Note

Currently, the VTR online documentation is built automatically based on the .readthedocs.yaml through Read the Docs Services. The build service (independent from GitHub Actions CI) is triggered whenever there is a push to the master branch.

Therefore, skipping all the current GitHub CI workflows won't affect the current online documentation build.

AlexandreSinger commented 3 weeks ago

I am 100% in support of this change! I also agree that the CI should not run if there are non-functional changes.

A potential issue you may run into is that there are required tests in our PR flow (sets of tests which must succeed in order for the code to be merged in). If the CI does not run for documentation changes, the PR may never be merged since it did not pass the required tests. I am not sure the best way to resolve this issue, but it should be resolved.

Also, maybe GitHub handles this on its own! I would not be surprised if there are some checks in place to allow PRs to merge if they specifically request not to run!

vaughnbetz commented 2 weeks ago

Some tests won't run, so we'll have to merge despite that. Let's go with that for now ... too bad we can't mark the docs as needing a smaller list of checks.