Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul diff-shades integration to fail on stable changes and preview changes for all projects #2951

Merged
merged 2 commits into from
Mar 26, 2022
Merged

Overhaul diff-shades integration to fail on stable changes and preview changes for all projects #2951

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
58ec056
Enforce no formatting changes for PRs via CI
ichard26 Mar 12, 2022
f045523
Merge branch 'main' into overhaul-diff-shades-integration
ichard26 Mar 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 59 additions & 55 deletions .github/workflows/diff_shades.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,54 +3,61 @@ name: diff-shades
on:
push:
branches: [main]
paths-ignore: ["docs/**", "tests/**", "**.md", "**.rst"]
paths: ["src/**", "setup.*", "pyproject.toml", ".github/workflows/*"]

pull_request:
paths-ignore: ["docs/**", "tests/**", "**.md", "**.rst"]

workflow_dispatch:
inputs:
baseline:
description: >
The baseline revision. Pro-tip, use `.pypi` to use the latest version
on PyPI or `.XXX` to use a PR.
required: true
default: main
baseline-args:
description: "Custom Black arguments (eg. -l 79)"
required: false
target:
description: >
The target revision to compare against the baseline. Same tip applies here.
required: true
target-args:
description: "Custom Black arguments (eg. -S)"
required: false
paths: ["src/**", "setup.*", "pyproject.toml", ".github/workflows/*"]

concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

jobs:
configure:
runs-on: ubuntu-latest
outputs:
matrix: ${{ steps.set-config.outputs.matrix }}

steps:
- uses: actions/checkout@v3
- uses: actions/setup-python@v3

- name: Install diff-shades and support dependencies
run: |
python -m pip install click packaging urllib3
python -m pip install https://github.com/ichard26/diff-shades/archive/stable.zip

- name: Calculate run configuration & metadata
id: set-config
env:
GITHUB_TOKEN: ${{ github.token }}
run: >
python scripts/diff_shades_gha_helper.py config ${{ github.event_name }} ${{ matrix.mode }}

analysis:
name: analysis / linux
name: analysis / ${{ matrix.mode }}
needs: configure
runs-on: ubuntu-latest
env:
# Clang is less picky with the C code it's given than gcc (and may
# generate faster binaries too).
CC: clang-12
strategy:
fail-fast: false
matrix:
include: ${{ fromJson(needs.configure.outputs.matrix )}}

steps:
- name: Checkout this repository (full clone)
uses: actions/checkout@v3
with:
# The baseline revision could be rather old so a full clone is ideal.
fetch-depth: 0

- uses: actions/setup-python@v3

- name: Install diff-shades and support dependencies
run: |
python -m pip install pip --upgrade
python -m pip install https://github.com/ichard26/diff-shades/archive/stable.zip
python -m pip install click packaging urllib3
python -m pip install -r .github/mypyc-requirements.txt
Expand All @@ -59,92 +66,89 @@ jobs:
git config user.name "diff-shades-gha"
git config user.email "diff-shades-gha@example.com"

- name: Calculate run configuration & metadata
id: config
env:
GITHUB_TOKEN: ${{ github.token }}
run: >
python helper.py config ${{ github.event_name }}
${{ github.event.inputs.baseline }} ${{ github.event.inputs.target }}
--baseline-args "${{ github.event.inputs.baseline-args }}"

- name: Attempt to use cached baseline analysis
id: baseline-cache
uses: actions/cache@v2.1.7
with:
path: ${{ steps.config.outputs.baseline-analysis }}
key: ${{ steps.config.outputs.baseline-cache-key }}
path: ${{ matrix.baseline-analysis }}
key: ${{ matrix.baseline-cache-key }}

- name: Build and install baseline revision
if: steps.baseline-cache.outputs.cache-hit != 'true'
env:
GITHUB_TOKEN: ${{ github.token }}
run: >
${{ steps.config.outputs.baseline-setup-cmd }}
${{ matrix.baseline-setup-cmd }}
&& python setup.py --use-mypyc bdist_wheel
&& python -m pip install dist/*.whl && rm build dist -r

- name: Analyze baseline revision
if: steps.baseline-cache.outputs.cache-hit != 'true'
run: >
diff-shades analyze -v --work-dir projects-cache/
${{ steps.config.outputs.baseline-analysis }} -- ${{ github.event.inputs.baseline-args }}
${{ matrix.baseline-analysis }} ${{ matrix.force-flag }}

- name: Build and install target revision
env:
GITHUB_TOKEN: ${{ github.token }}
run: >
${{ steps.config.outputs.target-setup-cmd }}
${{ matrix.target-setup-cmd }}
&& python setup.py --use-mypyc bdist_wheel
&& python -m pip install dist/*.whl

- name: Analyze target revision
run: >
diff-shades analyze -v --work-dir projects-cache/
${{ steps.config.outputs.target-analysis }} --repeat-projects-from
${{ steps.config.outputs.baseline-analysis }} -- ${{ github.event.inputs.target-args }}
${{ matrix.target-analysis }} --repeat-projects-from
${{ matrix.baseline-analysis }} ${{ matrix.force-flag }}

- name: Generate HTML diff report
run: >
diff-shades --dump-html diff.html compare --diff --quiet
${{ steps.config.outputs.baseline-analysis }} ${{ steps.config.outputs.target-analysis }}
diff-shades --dump-html diff.html compare --diff
${{ matrix.baseline-analysis }} ${{ matrix.target-analysis }}

- name: Upload diff report
uses: actions/upload-artifact@v2
with:
name: diff.html
name: ${{ matrix.mode }}-diff.html
path: diff.html

- name: Upload baseline analysis
uses: actions/upload-artifact@v2
with:
name: ${{ steps.config.outputs.baseline-analysis }}
path: ${{ steps.config.outputs.baseline-analysis }}
name: ${{ matrix.baseline-analysis }}
path: ${{ matrix.baseline-analysis }}

- name: Upload target analysis
uses: actions/upload-artifact@v2
with:
name: ${{ steps.config.outputs.target-analysis }}
path: ${{ steps.config.outputs.target-analysis }}
name: ${{ matrix.target-analysis }}
path: ${{ matrix.target-analysis }}

- name: Generate summary file (PR only)
if: github.event_name == 'pull_request'
if: github.event_name == 'pull_request' && matrix.mode == 'preview-changes'
run: >
python helper.py comment-body
${{ steps.config.outputs.baseline-analysis }} ${{ steps.config.outputs.target-analysis }}
${{ steps.config.outputs.baseline-sha }} ${{ steps.config.outputs.target-sha }}
${{ matrix.baseline-analysis }} ${{ matrix.target-analysis }}
${{ matrix.baseline-sha }} ${{ matrix.target-sha }}
${{ github.event.pull_request.number }}

- name: Upload summary file (PR only)
if: github.event_name == 'pull_request'
if: github.event_name == 'pull_request' && matrix.mode == 'preview-changes'
uses: actions/upload-artifact@v2
with:
name: .pr-comment.json
path: .pr-comment.json

# This is last so the diff-shades-comment workflow can still work even if we
# end up detecting failed files and failing the run.
- name: Check for failed files in both analyses
- name: Verify zero changes (PR only)
if: matrix.mode == 'assert-no-changes'
run: >
diff-shades compare --check ${{ matrix.baseline-analysis }} ${{ matrix.target-analysis }}
|| (echo "Please verify you didn't change the stable code style unintentionally!" && exit 1)

- name: Check for failed files for target revision
# Even if the previous step failed, we should still check for failed files.
if: always()
run: >
diff-shades show-failed --check --show-log ${{ steps.config.outputs.baseline-analysis }};
diff-shades show-failed --check --show-log ${{ steps.config.outputs.target-analysis }}
diff-shades show-failed --check --show-log ${{ matrix.target-analysis }}
--check-allow 'sqlalchemy:test/orm/test_relationship_criteria.py'
50 changes: 27 additions & 23 deletions docs/contributing/gauging_changes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,46 +9,50 @@ enough to cause frustration to projects that are already "black formatted".

## diff-shades

diff-shades is a tool that runs _Black_ across a list of Git cloneable OSS projects
recording the results. The main highlight feature of diff-shades is being able to
compare two revisions of _Black_. This is incredibly useful as it allows us to see what
exact changes will occur, say merging a certain PR.
diff-shades is a tool that runs _Black_ across a list of open-source projects recording
the results. The main highlight feature of diff-shades is being able to compare two
revisions of _Black_. This is incredibly useful as it allows us to see what exact
changes will occur, say merging a certain PR.

For more information, please see the [diff-shades documentation][diff-shades].

### CI integration

diff-shades is also the tool behind the "diff-shades results comparing ..." /
"diff-shades reports zero changes ..." comments on PRs. The project has a GitHub Actions
workflow which runs diff-shades twice against two revisions of _Black_ according to
these rules:
workflow that analyzes and compares two revisions of _Black_ according to these rules:

| | Baseline revision | Target revision |
| --------------------- | ----------------------- | ---------------------------- |
| On PRs | latest commit on `main` | PR commit with `main` merged |
| On pushes (main only) | latest PyPI version | the pushed commit |

Once finished, a PR comment will be posted embedding a summary of the changes and links
to further information. If there's a pre-existing diff-shades comment, it'll be updated
instead the next time the workflow is triggered on the same PR.
For pushes to main, there's only one analysis job named `preview-changes` where the
preview style is used for all projects.

The workflow uploads 3-4 artifacts upon completion: the two generated analyses (they
have the .json file extension), `diff.html`, and `.pr-comment.json` if triggered by a
PR. The last one is downloaded by the `diff-shades-comment` workflow and shouldn't be
downloaded locally. `diff.html` comes in handy for push-based or manually triggered
runs. And the analyses exist just in case you want to do further analysis using the
collected data locally.
For PRs they get one more analysis job: `assert-no-changes`. It's similar to
`preview-changes` but runs with the stable code style. It will fail if changes were
made. This makes sure code won't be reformatted again and again within the same year in
accordance to Black's stability policy.

Note that the workflow will only fail intentionally if while analyzing a file failed to
Additionally for PRs, a PR comment will be posted embedding a summary of the preview
changes and links to further information. If there's a pre-existing diff-shades comment,
it'll be updated instead the next time the workflow is triggered on the same PR.

```{note}
The `preview-changes` job will only fail intentionally if while analyzing a file failed to
format. Otherwise a failure indicates a bug in the workflow.
```

```{tip}
Maintainers with write access or higher can trigger the workflow manually from the
Actions tab using the `workflow_dispatch` event. Simply select "diff-shades"
from the workflows list on the left, press "Run workflow", and fill in which revisions
and command line arguments to use.
The workflow uploads several artifacts upon completion:

Once finished, check the logs or download the artifacts for local use.
```
- The raw analyses (.json)
- HTML diffs (.html)
- `.pr-comment.json` (if triggered by a PR)

The last one is downloaded by the `diff-shades-comment` workflow and shouldn't be
downloaded locally. The HTML diffs come in handy for push-based where there's no PR to
post a comment. And the analyses exist just in case you want to do further analysis
using the collected data locally.

[diff-shades]: https://github.com/ichard26/diff-shades#readme
Loading