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

use GitHub actions for linting #1616

Closed
wants to merge 4 commits into from
Closed

use GitHub actions for linting #1616

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a120ed4
action: use pre-commit
v1v Aug 18, 2022
e825b3e
action: run always for testing purposes
v1v Aug 18, 2022
9c0b32f
Revert "action: run always for testing purposes"
v1v Aug 18, 2022
e3e2818
Update .github/workflows/pre-commit.yml
v1v Aug 18, 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
44 changes: 0 additions & 44 deletions .ci/jobs/apm-agent-python-linting-mbp.yml
View file
Open in desktop

This file was deleted.

55 changes: 0 additions & 55 deletions .ci/linting.groovy
View file
Open in desktop

This file was deleted.

27 changes: 27 additions & 0 deletions .github/workflows/pre-commit.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
name: pre-commit

on:
pull_request:

permissions:
contents: read
pull-requests: read

jobs:
pre-commit:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/setup-python@v3
- uses: pre-commit/action@646c83fcd040023954eafda54b4db0192ce70507
with: ## let's skip to run for all the files
extra_args: --help
- id: files
uses: jitterbit/get-changed-files@b17fbb00bdc0c0f63fcf166580804b4d2cdc2a42
- name: Configure PATH
run: echo "${GITHUB_WORKSPACE}/.ci/scripts" >> $GITHUB_PATH
v1v marked this conversation as resolved.
Show resolved Hide resolved
- name: Precommit changes
run: |
for changed_file in ${{ steps.files.outputs.all }}; do
pre-commit run --files "${changed_file}"
done
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could not find a better way to run pre-commit for the subset of changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I'd be fine with running pre-commit on everything. It should be very fast. But this also seems fine 🤷‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this also work without the for loop?

pre-commit run --files ${{ steps.files.outputs.all }}

pre-commit run --files can take a list of files. Not sure if we could run into some limit if the change affects hundreds of files, though (e.g. when Python 4.0 moves from spaces to tabs ;) )

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, I'll try it in a bit :)