Code style

[lietu]

Was quickly checking the repository and noticed my IDE lighting up in red with various style issues everywhere. You're not following PEP-8 and other best practices of Python, which will limit the collaboration you will get as it's painful for anyone to follow an unpredictable and abnormal style.

My personal recommendation is to set up pre-commit with hooks for

• ruff to lint the code, auto-sort imports, and auto-format the code
• bandit to scan the code for security issues
• trailing-whitespace to remove .. trailing whitespace
• end-of-file-fixer to add an empty line at the end of files
• mixed-line-ending to convert all .py files (and probably most other files in the project) to Unix/LF/\n -line endings from Windows/CRLF/\r\n -line endings

as well as

• skjold to perform automatic security audit of dependencies - but for this you should also migrate to proper use of poetry for actual dependency management instead of the archaic requirements.txt

A /.pre-commit-config.yaml with contents similar to this should improve things a lot if taken into use:

repos:
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.6.0
    hooks:
      - id: trailing-whitespace
        exclude: |
          (?x)^(
            .*Altium_Source_Files.*|
            .*\.stl$|
            .*\.step$|
            ^NOT-EXISTING-LAST-ENTRY$
          )$
      - id: end-of-file-fixer
        exclude: |
          (?x)^(
            .*Altium_Source_Files.*|
            .*\.stl$|
            .*\.step$|
            ^NOT-EXISTING-LAST-ENTRY$
          )$

      # All non-bat files should end with LF unless generated or external etc.
      - id: mixed-line-ending
        name: Ensure LF endings on most files
        args: ["--fix=lf"]
        exclude: |
          (?x)^(
            \.bat$|
            .*Altium_Source_Files.*|
            .*\.stl$|
            .*\.step$|
            ^NOT-EXISTING-LAST-ENTRY$
          )$
      # Bat files should end with CRLF
      - id: mixed-line-ending
        name: Ensure CFLF endings on Windows files
        args: ["--fix=crlf"]
        files: \.bat$

  - repo: https://github.com/astral-sh/ruff-pre-commit
    rev: v0.5.4
    hooks:
      - id: ruff
        name: ruff lint
        args: [--fix]
      - id: ruff
        name: ruff sort imports
        args: ["--select", "I", "--fix"]
      - id: ruff-format
        name: ruff format

#  # You can probably only benefit from skjold vulnerability scans if using poetry for dependency management
#  - repo: https://github.com/twu/skjold
#    rev: v0.6.2
#    hooks:
#      - id: skjold
#        verbose: true

  - repo: https://github.com/PyCQA/bandit
    rev: 1.7.9
    hooks:
      - id: bandit
        args:
          - --configfile
          - bandit.yaml
        exclude: |
          (?x)^(
            .*/tests/.*|
            ^NOT-EXISTING-LAST-ENTRY$
          )$

And /bandit.yaml with the contents:

assert_used:
  skips: ["*/test_*.py", "*/test_*.py"]

This is how it could look like: https://github.com/lietu/OpenFanController/tree/chore/precommit_autoformat or for a diff https://github.com/lietu/OpenFanController/commit/081cc0a7efef90c407780e90aecb371ea8899e6d but most of it is line endings so I'd suggest checking the "ignore whitespace" -version for issues with your Python code formatting that can easily be fixed with the automation

If you look around a bit you can probably find similar autoformatting utilities for the C -side which I can only assume suffers from the same issue of an unpredictable and uncommon code style considering I immediately saw unpredictable naming styles for variables when I opened fan_control.c and even filenames seem to suffer from lack of predictability.

Is this something you'd be interested in integrating ot the repo?

[SasaKaranovic]

Thanks @lietu ! I will keep this in mind for future development. However this is not a priority right now.

[github-actions[bot]]

This issue is stale because it has been open 15 days with no activity. Remove stale label or comment or this will be closed in 3 days.

[github-actions[bot]]

This issue was closed because it has been stalled for 5 days with no activity.