ITensor / ITensors.jl

A Julia library for efficient tensor computations and tensor network calculations
https://itensor.org
Apache License 2.0
522 stars 122 forks source link

[ENHANCEMENT] Improve format Github action #803

Closed mtfishman closed 2 years ago

mtfishman commented 2 years ago

We could try this Github action for formatting:

name: Format suggestions

on:
  pull_request:

concurrency:
  # Skip intermediate builds: always.
  # Cancel intermediate builds: only if it is a pull request build.
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: ${{ startsWith(github.ref, 'refs/pull/') }}

jobs:
  format:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - uses: julia-actions/setup-julia@latest
        with:
          version: 1
      - run: |
          julia  -e 'using Pkg; Pkg.add("JuliaFormatter")'
          julia  -e 'using JuliaFormatter; format("."; verbose=true)'
      - uses: reviewdog/action-suggester@v1
        with:
          tool_name: JuliaFormatter
          fail_on_error: true
          filter_mode: added

used by ChainRulesCore.jl, which I think actually makes suggestions for formatting issues to change on the PR (whereas ours currently just says if there is a formatting issue, but doesn't suggest a change to make).

emstoudenmire commented 2 years ago

I was actually looking the other week to see if there was a way to do this. Would be good to have.

Could the action just run the formatter no matter what? Because in any case when the formatting is wrong, the next step is always to just run the formatter. Or I guess is the issue purely technical about needing to make a new commit after the formatter runs?

mtfishman commented 2 years ago

I was hoping that is what the above Github Action will do (make a commit to the PR with proposed changes to fix the formatting), but I haven't tested it.

Currently, the Github Action (https://github.com/ITensor/ITensors.jl/blob/8f8229a3ed56012a0252a30f49a421f157bf338a/.github/workflows/format_check.yml) does run no matter what (at every commit to a PR or the main branch), it just doesn't propose a change or make a new commit with formatting changes.

We also have this Github Action (https://github.com/ITensor/ITensors.jl/blob/8f8229a3ed56012a0252a30f49a421f157bf338a/.github/workflows/format_pr.yml) which makes a new PR periodically with formatting fixes. But it would nice if these behaviors were combined into a single action that directly aids when making a PR.

emstoudenmire commented 2 years ago

Yes, let's try these out. It would be great if every PR just had the formatter run on it and a new commit created automatically if the formatter results in a change.

mtfishman commented 2 years ago

I just realized that behavior could become annoying when developing.

For example, say you push a commit to a PR you are working on. Then the format action pushes it's own commit to fix a formatting issue you introduced. You continue developing and happen to work on the same lines the format action edited. Now the next time you try to commit, you suddenly have a bunch of conflicts...

mtfishman commented 2 years ago

Maybe a better way would be to have a link on PRs on Github somewhere where you can manually make a commit that fixes any formatting issues.

mtfishman commented 2 years ago

Closed by #811.