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.

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

dropping pre-commit.ci #37

Open
dlqqq opened this issue Sep 15, 2022 · 1 comment
Open

dropping pre-commit.ci #37

dlqqq opened this issue Sep 15, 2022 · 1 comment
Labels
bug Something isn't working

Comments

@dlqqq
Copy link

dlqqq commented Sep 15, 2022

cc @blink1073

In the Github Actions config used by Jupyter Server and the server extension cookiecutter, we are already including a pre-commit step where we are invoking pre-commit run --all-files --hook-stage=manual. This was weird to me because I had expected this to be done by pre-commit.ci.

Playing around with the hook stages seemed to demonstrate why we were running pre-commit ourselves in tandem with pre-commit.ci. The pre-commit step

  - repo: https://github.com/sirosen/check-jsonschema
    rev: 0.15.0
    hooks:
      - id: check-jsonschema
        name: "Check GitHub Workflows"
        files: ^\.github/workflows/
        types: [yaml]
        args: ["--schemafile", "https://json.schemastore.org/github-workflow"]
        stages: [manual]

makes a network request to a JSON schema registry, which is forbidden by pre-commit.ci. Hence we isolate this step in a separate hook stage (manual) and then invoke pre-commit with that hook stage in our own Github Actions CI.

Which got me thinking: can we just drop pre-commit.ci since we have to run it ourselves anyways? I'm not sure how it's being included, but I suspect it's an org-level setting where all repos created under jupyter-server have pre-commit.ci included in its pipeline.

Dropping pre-commit.ci, along with #36, actually lets us drop the "manual" hook stage for all pre-commit steps, which is a big developer win. This will ensure pre-commit never fails exclusively in CI while passing locally (when hook stage is unspecified), making development way friendlier.

The next steps are similar to those of #36. Remove pre-commit.ci from Jupyter server, the server cookiecutter extension, and all other server extensions.

@dlqqq dlqqq added the bug Something isn't working label Sep 15, 2022
@blink1073
Copy link
Contributor

We use the manual stage for things that cannot be fixed automatically as well, like flake8. We don't want to block pushing code just because a linter fails, but we do want to block merging it. We also use pre-commit.ci for the auto-update of the pre-commit dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants