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

Streamline Local Execution of Pre-commit Tooling and Simplify Developer Workflow #7344

Open
jcfr opened this issue Nov 3, 2023 · 0 comments
Labels
Type: Enhancement Improvement to functionality

Comments

@jcfr
Copy link
Member

jcfr commented Nov 3, 2023

Is your feature request related to a problem? Please describe.

Currently, Slicer developers are required to call the script Utilities/SetupForDevelopment.sh before crafting local commits and creating a branch that will be contributed as a pull request. This script performs various tasks, including checking if the Git environment is configured1, configuring git hooks2, and providing developer tips3.

However, once a pull request is created, multiple GitHub Workflows are triggered to perform operations like building Slicer (CI (Build) workflow) and running pre-commit hooks (CI (Lint) workflow) defined in the .pre-commit-config.yaml file.

hooks:
- id: check-added-large-files
args: ['--maxkb=1024']
- id: check-case-conflict
- id: check-merge-conflict
- id: check-symlinks
- id: trailing-whitespace
exclude: "\\.(svg|vtk|vtp)$"
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.1.3
hooks:
- id: ruff
args: ["--fix", "--show-fixes"]

The issue lies in the differences between the checks executed through the CI (Lint) workflow and those configured through SetupForDevelopment.sh. Additionally, developers can install pre-commit and run it locally in a Python virtual environment, but this requires configuring the virtual environment.

Problems:

  1. Inconsistencies between CI (Lint) checks and those configured in SetupForDevelopment.sh.
  2. The need to manually set up a virtual environment for running pre-commit4 or ruff.
  3. Configuration complexity for developers.

Describe the solution you'd like

To streamline the developer workflow and remove the need for explicit virtual environment setup, consider the following steps:

  • Update SetupForDevelopment.sh to check if a local installation of Python exists and ask if the developer wants to create a virtual environment called Slicer/.git/.venv-slicer-build.

    • If such an environment can be created, ask the developer if they would like to run pre-commit install5. This ensures that hooks (e.g., linting and reformatting) are executed locally during commits.
  • If there is no Slicer/.git/.venv-slicer-build environment, create it instead in ~/Slicer-SuperBuild/.venv-slicer-build, but do not install pre-commit, as the build tree may not exist.

Regardless of the .venv-slicer-build location:

  • Install the latest stable versions of the pre-commit and ruff Python packages.

  • Add convenience targets named RunPreCommit and RunRuff to both the SuperBuild and the inner build (excluding them from ALL) and add tests checking they can run successfully.

Describe alternatives you've considered

NA

Additional context

Originally posted by @lassoan in #7338 (comment)

[...] What I would really appreciate though if it was possible to:

  • easily run the exact same checks in the local build tree without any extra configuration, maybe as part of the build process
  • have an option for the the linter to change the source code to make it compliant

Footnotes

  1. https://github.com/Slicer/Slicer/blob/main/Utilities/Scripts/SetupUser.sh

  2. https://github.com/Slicer/Slicer/blob/main/Utilities/Scripts/SetupHooks.sh

  3. https://github.com/Slicer/Slicer/blob/main/Utilities/Scripts/GitTips.sh

  4. Locally running the pre-commit hook is possible using pre-commit run --a (or pre-commit run --all-files or pre-commit run --all-files)

  5. https://pre-commit.com/#pre-commit-install

@jcfr jcfr added the Type: Enhancement Improvement to functionality label Nov 3, 2023
@jcfr jcfr changed the title Streamline execution of pre-commit tooling from a build tree. Streamline local execution of pre-commit tooling Nov 3, 2023
@jcfr jcfr changed the title Streamline local execution of pre-commit tooling Streamline Local Execution of Pre-commit Tooling and Simplify Developer Workflow Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Improvement to functionality
Development

No branches or pull requests

1 participant