Skip to content

Conversation

@sgonorov
Copy link
Contributor

Minimal pre-commit configuration and CI integration.

For now only fixes whitespaces/end-of-file and some other small checks.

Will be used a base for adding other linters.

For the maxed-out Formatting/Linting see PR: #2980

@sgonorov sgonorov self-assigned this Nov 13, 2025
@github-actions github-actions bot added category: GHA CI based on Github actions no-match-files labels Nov 13, 2025
@@ -0,0 +1,20 @@
repos:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Am I right that it would be required to fix linting issues for any changed files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, only for changed ones.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So If I change 2 lines in several hundred loc file I would be required to fix lint for the whole file. Then feature changes would endup hidden in the linting changes. Is it correct?

Copy link
Contributor Author

@sgonorov sgonorov Nov 13, 2025

Choose a reason for hiding this comment

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

Yes, that's the way. We can't partially format file sadly.
But for now it's mostly whitespaces as a starting point so it won't affect much. Later I'll try to make a separate PRs with properly formatted files and new formatters/linters introduction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, for python there's a formatter called Darker which is compatible with Ruff format, but applies changes only to changed lines. When we get to it, maybe we'll use it instead of simply enabling Ruff across the codebase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It ignores python files. I can add CPP-related file to exception for now too if needed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

What "it" refers to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whitespace hook - for now it ignores only Pyhton files. We can add cpp files also.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Add cpp to fix #3008 (comment) Any reason not to do so?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added. Then i'll create some migratiion PRs where fix all the whitespaces and remove these ignores.

@Wovchena Wovchena requested a review from Copilot November 13, 2025 09:59
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 introduces a minimal pre-commit configuration for the repository with automated linting in CI. The implementation focuses on basic code hygiene checks like whitespace handling, file endings, and security validations, serving as a foundation for future linting tools.

Key Changes:

  • Added pre-commit configuration with standard hooks for whitespace, file endings, and basic validation
  • Created GitHub Actions workflow to run pre-commit checks on changed files in pull requests

Reviewed Changes

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

File Description
.pre-commit-config.yaml Defines pre-commit hooks for trailing whitespace, end-of-file fixing, merge conflicts, symlinks, private keys, line endings, and file format validation
.github/workflows/lint.yml Implements CI workflow that downloads OpenVINO, sets up dependencies, and runs pre-commit checks on changed files

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

@github-actions github-actions bot added the category: cmake / build Cmake scripts label Nov 13, 2025
@sgonorov sgonorov force-pushed the precommit_min_implementation branch from 3f909bd to 3248f48 Compare November 14, 2025 09:25
@sgonorov sgonorov requested a review from as-suvorov November 14, 2025 09:26
@sgonorov sgonorov force-pushed the precommit_min_implementation branch from 3248f48 to a2bb03f Compare November 17, 2025 18:58
pyproject.toml Outdated
build-backend = "py_build_cmake.build"

[tool.ruff]
line-length = 100
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why we should also follow 160 line-length rule? Are there any specific reasons? I consider avoiding it as much as possible - such a high value defeats formatting purpose and makes code much harder to read.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Consistency. You can ask OV to change that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

openvinotoolkit/openvino#32987 - will change to 120, it's not perfect but it's much better. I'll just wait for this PR to be merged.

Copy link
Contributor Author

@sgonorov sgonorov Nov 26, 2025

Choose a reason for hiding this comment

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

Change implemented in OV - now line length equals 120. I've also updated the PR.

@sgonorov sgonorov force-pushed the precommit_min_implementation branch 2 times, most recently from a133d96 to 0172e7e Compare November 26, 2025 12:55
@Wovchena Wovchena requested a review from Copilot November 26, 2025 13:08
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

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


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

@sgonorov sgonorov force-pushed the precommit_min_implementation branch from 0172e7e to c44cb7b Compare December 3, 2025 10:29
@Wovchena Wovchena enabled auto-merge December 3, 2025 10:46
@Wovchena Wovchena requested a review from Copilot December 3, 2025 10:46
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

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


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

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

Labels

category: cmake / build Cmake scripts category: GHA CI based on Github actions no-match-files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants