Skip to content

Pre-commit Linter is Inconsistent in Enforcing Code Formatting #81

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

Open
dPys opened this issue Sep 22, 2024 · 2 comments
Open

Pre-commit Linter is Inconsistent in Enforcing Code Formatting #81

dPys opened this issue Sep 22, 2024 · 2 comments

Comments

@dPys
Copy link
Contributor

dPys commented Sep 22, 2024

Description
It seems that the pre-commit hooks aren't enforcing linting consistently, as some code style changes were missed, even though the pre-commit checks passed.

Specifically, the following function in init:

def __init__(
    self,
    graph_object,
):

was not flagged for formatting, but it should have been a one-liner. The relevant pre-commit hooks (black, ruff, etc.) passed successfully despite this inconsistency.

Steps to Reproduce
Make a formatting change like the above.
Run the following pre-commit command:

pre-commit run --all-files --show-diff-on-failure --color always

Observe that the hooks pass, even though the code style is not consistent.

Expected Behavior
The pre-commit hook should have failed, flagging the multi-line formatting in the init function and enforcing a consistent code style.

Actual Behavior
The pre-commit hook passed without flagging the inconsistent formatting.

Environment
Pre-commit configuration:
blacken-docs: Passed
prettier: Passed
ruff: Passed
ruff-format: Passed
IDE: Possible IDE-related changes may also be contributing to this issue, but pre-commit should catch and enforce the linting regardless.

Proposed Fix
We may want to:
Review and refine the pre-commit configuration to ensure consistent enforcement of linting, especially with formatting rules.
Align the current repo's pre-commit hooks with the ones used in the nx-parallel package for uniformity across all repositories.
Update the versions of both ruff and black in the .pre-commit-config.yaml file, as they are currently outdated:
Upgrade ruff to at least v0.5.1 (currently using v0.1.8).
Upgrade black to at least v24.3.0 (to ensure up-to-date formatting standards).
Confirm that the pre-commit checks properly catch multi-line issues like this in the future, and make necessary adjustments to hook configurations.

Other considerations
We might consider also adding:
Fixing end-of-file issues (end-of-file-fixer).
Fixing trailing whitespaces (trailing-whitespace)

This would look as follows:

-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.3.0
    hooks:
    -   id: check-yaml
    -   id: end-of-file-fixer
    -   id: trailing-whitespace
@MridulS
Copy link
Member

MridulS commented Sep 25, 2024

This may have fixed this c73066f, we were installing a ruff inside the env and one in the pre-commit env :)

@Schefflera-Arboricola
Copy link
Member

This may have fixed this c73066f, we were installing a ruff inside the env and one in the pre-commit env :)

But, the version of both the ruffs was same.

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

No branches or pull requests

3 participants