Skip to content

Conversation

@jonpspri
Copy link
Contributor

@jonpspri jonpspri commented Oct 25, 2025

Summary

  • Enable flake8 and pylint linters in Neovim ALE configuration
  • Enable flake8 pre-commit hook for automated code quality checks
  • Remove obsolete fix-encoding-pragma hook (Python 3+ doesn't require encoding declarations)

Benefits

  • Consistent linting feedback during development (IDE) and pre-commit
  • Earlier detection of style issues before CI runs
  • Unified code quality enforcement across development workflow

Test plan

  • Verify flake8 runs in Neovim when editing Python files
  • Verify pylint runs in Neovim when editing Python files
  • Confirm pre-commit run flake8 executes successfully
  • Verify encoding pragma is no longer added to Python files

Copilot AI review requested due to automatic review settings October 25, 2025 07:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances Python code quality enforcement by enabling flake8 and pylint linters in both the development environment (Neovim) and the pre-commit pipeline. The changes also remove the obsolete fix-encoding-pragma hook, which is unnecessary for Python 3+.

Key Changes

  • Enabled flake8 and pylint in Neovim's ALE linter configuration for real-time feedback during development
  • Uncommented and activated the flake8 pre-commit hook to catch issues before commits
  • Removed the deprecated fix-encoding-pragma hook that added UTF-8 encoding declarations

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
.pre-commit-config.yaml Enabled flake8 pre-commit hook and removed obsolete encoding pragma fixer
.nvim.lua Added flake8 and pylint to ALE linters for Python files

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jonpspri jonpspri force-pushed the chore/flake8-plint-in-neovim-ide branch 3 times, most recently from becddb8 to 5225375 Compare October 25, 2025 09:03
@jonpspri
Copy link
Contributor Author

Pylint rolled out v4.x overnight, which we're not able to use because of other dependencies. So I reworked the GitHub CI/CD to use our dependencies (pyproject.toml) to determine the pylint to install and use. Thanks, uv!

@crivetimihai
Copy link
Member

I'd rather not have flake8, pylint, etc. as part of pre-commit. Pre-commit has most of the cleanup tasks that change indentation, non-utf characters, etc.

Having pylint and flake8 as part of pre-commit takes too long.

@jonpspri
Copy link
Contributor Author

jonpspri commented Oct 27, 2025

I'd rather not have flake8, pylint, etc. as part of pre-commit. Pre-commit has most of the cleanup tasks that change indentation, non-utf characters, etc.

Having pylint and flake8 as part of pre-commit takes too long.

My thinking is that pre-commit when running as a commit hook (how I most rely on it) runs only on the files being committed, so placing them there presents a gatepost for incremental code improvement, given reasonable rules definition. Their execution time is order-n on the files being scanned, so for properly-sized commits the increase is negligible. That's the use case I'm trying to address.

And I HATE getting whacked in the GitHub lint workflows for something that could have been caught in pre-commit.

Having said that, I think we need to steer clear of using 'pre-commit run --all-files' as part of a regular commit or CI/CD lifecycle. That's something that can be controlled from the Makefile and the GitHub workflows independently.

(Let me know if I need to do some "typical commit" benchmarking to convince you.)

@jonpspri
Copy link
Contributor Author

@crivetimihai Perhaps two commit files?

  1. One with as much as possible of what the GitHub actions will test to support actual pre-commit hooks and reject commits that will just fail PR. This one can include pylint and flake8 for example, but would generally be used only on files actually changed in a commit.
  2. A second lightweight version with the hooks that are unique to pre-commit to be used in the Makefile with the --all-files option. The '--config' option supports this.

jonpspri and others added 7 commits October 28, 2025 18:49
The pre-commit target was using 'uv pre-commit run' which is not a valid
uv subcommand. Changed to 'uv run pre-commit run' to properly invoke the
pre-commit tool through uv's run command.

Signed-off-by: Mihai Criveti <[email protected]>
@crivetimihai crivetimihai force-pushed the chore/flake8-plint-in-neovim-ide branch from f74a758 to a09067f Compare October 28, 2025 19:03
@crivetimihai crivetimihai merged commit 4262b1a into IBM:main Oct 28, 2025
44 checks passed
@jonpspri jonpspri deleted the chore/flake8-plint-in-neovim-ide branch October 29, 2025 10:00
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.

2 participants