Skip to content

feat: Added lint and format check to the CI/CD pipeline#1392

Open
Luka-D wants to merge 5 commits into
llm-d:mainfrom
Luka-D:feat-add-ci-lint-check
Open

feat: Added lint and format check to the CI/CD pipeline#1392
Luka-D wants to merge 5 commits into
llm-d:mainfrom
Luka-D:feat-add-ci-lint-check

Conversation

@Luka-D
Copy link
Copy Markdown
Contributor

@Luka-D Luka-D commented May 22, 2026

Part of #938, this change introduces a new workflow step where ruff is used to check whether the files within a PR pass the lint and format rules. Now with more verbosity for lint and format checks.

Added a linting and formatting check to the CI - PR workflow. This check will not modify any files, it will only check the files included in the PR and check whether they pass the lint and format standards.

Signed-off-by: Luka Dojcinovic <56648891+Luka-D@users.noreply.github.com>
@Luka-D
Copy link
Copy Markdown
Contributor Author

Luka-D commented May 22, 2026

@Vezio Can you run the workflows again please? I think the linting will fail again, but that's ok, I want to make sure the more verbose output works. After that I'll fix the lint errors and make another commit. Thank you 😃

Signed-off-by: Luka Dojcinovic <56648891+Luka-D@users.noreply.github.com>
@Luka-D
Copy link
Copy Markdown
Contributor Author

Luka-D commented May 26, 2026

Looks like there was an outage with actions: https://www.githubstatus.com/

@maugustosilva
Copy link
Copy Markdown
Collaborator

@Luka-D I have re-run all jobs (indeed there was a period of outage on GitHub), and it looks like a legitimate failure on ruff

@Luka-D
Copy link
Copy Markdown
Contributor Author

Luka-D commented May 29, 2026

Yes, that is the expected behavior. The current codebase has some lint errors that were detected by Ruff and so the workflow failed. I didn't configure the workflow to modify any of the files, so the expected resolution to this is for the PR owner to manually run pre-commit run ruff-check --all-files on their branch and commit the changes.

Failed lint errors:
image

Since the pre-commit change was merged recently, there are probably some lint errors that slipped through the cracks. I will make the necessary lint fixes on this branch and make another commit. The lint check should pass afterwards. Hopefully in the future all lint errors will be caugh by pre-commit before even reaching the CI/CD stage.

Lint errors either involved unused imports, having exception as e (if e is never used) or declaring variables which are never used (This error is skipped using # noqa: F841).

Signed-off-by: Luka Dojcinovic <56648891+Luka-D@users.noreply.github.com>
@Luka-D
Copy link
Copy Markdown
Contributor Author

Luka-D commented May 29, 2026

Alright, the lint check passed now, but the format check failed because there are quite a few files that are not formatted yet. I'll make a new branch where I'll format all the existing files in the code base. Once those are formatted, both checks should succeed. I apologize for the confusion.

@maugustosilva
Copy link
Copy Markdown
Collaborator

@Luka-D Not a problem at all. Thank you very much for the contributions!

@Luka-D
Copy link
Copy Markdown
Contributor Author

Luka-D commented May 29, 2026

My pleasure! Please see #1428 for part 1 of the formatting changes.

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