Skip to content

Conversation

@webknjaz
Copy link
Contributor

@webknjaz webknjaz commented Dec 28, 2024

Put an x into the box if that apply:

  • This PR introduces breaking change.
  • This PR fixes a bug.
  • This PR adds new functionality.
  • This PR enhances existing functionality.
  • This is a contributor/maintainer facing change that in no way affects the end-users

Description of your changes

Enabling faster feedback loop for new contributors by enabling https://pre-commit.ci in this repo.

The only visible effect of this is when said app is installed and the service is running the checks, it would skip the checks that rely on the external executables that might be missing in runtime. It's necessary since that testing environment is controlled by someone else.

How can we test changes

@antonbabenko go to https://github.com/apps/pre-commit-ci/installations/select_target, install the app into this repo and then merge the PR.

@webknjaz webknjaz changed the title 🧪 Disable checks unrunable @ pre-commit.ci ci: X🧪 Disable checks unrunable @ pre-commit.ci Dec 28, 2024
@MaxymVlasov
Copy link
Collaborator

We have pre-commit via GHA, and it runs all hooks specified in .pre-commit-config, not just some of them

@webknjaz
Copy link
Contributor Author

Alternatively, there's https://github.com/apps/pre-commit-ci-lite/installations/new + https://pre-commit.ci/lite.html that implements backpushes.

@yermulnik
Copy link
Collaborator

M-m-m, I definitely would suggest to add more context for other contributors, including a brief explanation added as a comment to the code that is changing for future reference and for posterity.

@webknjaz
Copy link
Contributor Author

Done.

@MaxymVlasov MaxymVlasov changed the title ci: X🧪 Disable checks unrunable @ pre-commit.ci ci: Disable checks unrunable @ pre-commit.ci Dec 28, 2024
Copy link
Collaborator

@MaxymVlasov MaxymVlasov left a comment

Choose a reason for hiding this comment

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

Copy link
Owner

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

@MaxymVlasov I have just installed github app as required

@webknjaz
Copy link
Contributor Author

pre-commit.ci run

@webknjaz
Copy link
Contributor Author

It's not marked as required in branch protection, though.

@antonbabenko
Copy link
Owner

@webknjaz Updated.
image

@webknjaz
Copy link
Contributor Author

This check is now failing in other PRs and will keep failing until this PR is merged.

webknjaz and others added 2 commits December 30, 2024 20:21
This patch drops the document start and end markers to comply with arbitrary personal preferences.

Co-authored-by: Maksym Vlasov <[email protected]>
@webknjaz webknjaz force-pushed the maintenance/pre-commit-ci branch from 0053f28 to 7672361 Compare December 30, 2024 19:21
@MaxymVlasov MaxymVlasov merged commit e5e882a into antonbabenko:master Dec 30, 2024
6 checks passed
@antonbabenko
Copy link
Owner

This PR is included in version 1.97.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants