feat: enhance golangci-lint configuration for better code quality #367

rishi-jat · 2025-10-04T14:33:02Z

Fixes #149

elevran

@rishi-jat thanks for the PR.
A few points that are required in order to start moving it forward:

DCO is required
rebasing is required
while helpful for PR review, the markdown files are not part of the configuration change and should not be included in the PR.
CI should pass (currently fails)
lint configuration file is missing a newline at end of file.

In addition, we'll review the list of added/modified linters` configuration so can reach consensus on specific linters ROI (e.g., false positive rates...)

elevran · 2025-10-24T07:38:31Z

@rishi-jat please address previous review comments (DCO, remove extra files in commit).
If you're unsure about a specific ask, let me know and I can help you address that.

rishi-jat · 2025-10-24T07:41:53Z

Apologies, For late will do all the suggested changes today! Thanks

elevran · 2025-10-26T09:57:54Z

thanks for removing the extra files.
The PR fails lint and test (invalid/missing version?) and that should be fixed.
Once that's fixed, we can merge and later fine-tune if needed (e.g., there might be overlap, gocritic tends to be too opinionated unless tuned, etc.)

- Add 10+ new high-value linters focusing on security, k8s patterns, and code quality - Enhanced error handling with errorlint, nilerr for robust controller code - Added security linters: gosec, bodyclose, contextcheck, noctx - Improved import organization with gci for large project maintainability - Added character safety checks: asciicheck, bidichk - Enhanced testing support with testpackage for Ginkgo conventions - Strategically disabled overly restrictive linters (varnamelen, exhaustruct, etc.) - Maintains compatibility with existing codebase while improving standards Fixes llm-d#149 Signed-off-by: Rishi Jat <rishijat098@gmail.com>

rishi-jat · 2025-10-30T14:02:24Z

@elevran I have fix the lint error and also can you please add Hacktoberfestt tag as this pr is part of my Hacktoberfest contribution!

elevran · 2025-10-31T12:41:20Z

what is an Hacktoberfest tag?
It's not an existing label we have. Is llm-d-inference-scheduler repo signed up for that?

elevran · 2025-11-04T15:52:40Z

.golangci.yml

@@ -1,33 +1,80 @@
-version: "2"


@rishi-jat "version: 2" is required and must be explicitly set (version 1 uses a different file format - see lint action failure on the PR).
Please restore this line

okay, will do ASAP.

@rishi-jat the latest commit is still missing the "version: 2" line and fails passing the required lint test.
Please fix and run make lint locally on your branch and ensure passing before resubmitting for review.
/hold

github-actions · 2025-11-26T01:36:26Z

This PR is marked as stale after 21d of inactivity. After an additional 14d of inactivity, it will be closed. To prevent this PR from being closed, add a comment or remove the lifecycle/stale label.

* Make sure that max_completion_tokens=1 in Prefill Signed-off-by: Shmuel Kallner <kallner@il.ibm.com> * Remove/undo setting of max_completion_tokens to 1, for decode Signed-off-by: Shmuel Kallner <kallner@il.ibm.com> --------- Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>

* elaborate relation to IGW/GIE Signed-off-by: Etai Lev Ran <elevran@gmail.com> * coalesce sections on relation to GIE Signed-off-by: Etai Lev Ran <elevran@gmail.com> --------- Signed-off-by: Etai Lev Ran <elevran@gmail.com>

- Remove deprecated gomnd linter - Add newline at end of file - Add exclusion rules for problematic packages - Configuration now works with modern golangci-lint versions Fixes golangci-lint compatibility and CI issues. Signed-off-by: Rishi Jat <rishijat098@gmail.com>

- Add required 'version: 2' field as requested in review - Move goimports and gofmt from linters to formatters section - Fix configuration structure to match golangci-lint v2 format - Maintain all original and new linters from previous enhancement Addresses feedback from maintainer review in llm-d#367 Signed-off-by: Rishi Jat <rishijat098@gmail.com>

- Remove version field temporarily to test with current CI setup - Fix incorrect version v2.1.6 in CI workflow - Use golangci-lint-action@v8 default version Signed-off-by: Rishi Jat <rishijat098@gmail.com>

Signed-off-by: Rishi Jat <rishijat098@gmail.com>

rishi-jat · 2025-11-26T02:51:36Z

@elevran PTAL

elevran · 2025-11-26T11:38:46Z

.github/workflows/ci-pr-checks.yaml

      - name: Run lint checks
        uses: golangci/golangci-lint-action@v9
        with:
-          version: 'v2.1.6'


what is the rationale for removing the specific version? will it run with some latest v2?

elevran · 2025-11-26T11:40:32Z

pkg/sidecar/proxy/proxy.go

should this change be part of this PR?
Does not seem lint related. Can you please remove it from the PR?

rishi-jat · 2025-11-28T11:20:48Z

@elevran please see PR: #495 Thanks!

rishi-jat · 2025-11-28T11:25:01Z

The PR had been open for quite a while. The issue itself was simple, but I unintentionally made it more complex earlier and couldn’t give it proper attention due to commitments elsewhere. I’ve now opened a clean PR that fixes the issue correctly, and it’s ready for review and merge. Sorry for the delay, and thank you for the patience.

elevran · 2025-11-28T11:56:02Z

@rishi-jat thank you for following up with #495 as replacement!

rishi-jat mentioned this pull request Oct 4, 2025

Review (and expand, if needed) golangci-lint configuration #149

Open

elevran added this to llm-d-inference-scheduler Oct 16, 2025

elevran moved this to In review in llm-d-inference-scheduler Oct 19, 2025

elevran requested changes Oct 20, 2025

View reviewed changes

elevran moved this from In review to In progress in llm-d-inference-scheduler Oct 20, 2025

rishi-jat force-pushed the enhance/golangci-lint-configuration branch from f32c095 to 33cbde7 Compare October 21, 2025 08:49

rishi-jat force-pushed the enhance/golangci-lint-configuration branch 3 times, most recently from fffc431 to 8c41d39 Compare October 25, 2025 22:39

rishi-jat force-pushed the enhance/golangci-lint-configuration branch from 8c41d39 to 0b021fc Compare October 28, 2025 17:43

shmuelk assigned rishi-jat Nov 3, 2025

elevran reviewed Nov 4, 2025

View reviewed changes

elevran moved this from In progress to In review in llm-d-inference-scheduler Nov 4, 2025

elevran moved this from In review to In progress in llm-d-inference-scheduler Nov 17, 2025

github-actions bot added the lifecycle/stale label Nov 26, 2025

shmuelk and others added 5 commits November 26, 2025 07:28

fix: update golangci-lint configuration and CI version

fa6854a

- Remove version field temporarily to test with current CI setup - Fix incorrect version v2.1.6 in CI workflow - Use golangci-lint-action@v8 default version Signed-off-by: Rishi Jat <rishijat098@gmail.com>

rishi-jat force-pushed the enhance/golangci-lint-configuration branch from 2a03593 to fa6854a Compare November 26, 2025 02:49

Merge branch 'main' into enhance/golangci-lint-configuration

ed98e38

Signed-off-by: Rishi Jat <rishijat098@gmail.com>

elevran requested changes Nov 26, 2025

View reviewed changes

github-project-automation bot moved this from In progress to In review in llm-d-inference-scheduler Nov 26, 2025

github-actions bot removed the lifecycle/stale label Nov 27, 2025

Merge branch 'llm-d:main' into enhance/golangci-lint-configuration

a1c13a1

rishi-jat closed this Nov 28, 2025

github-project-automation bot moved this from In review to Done in llm-d-inference-scheduler Nov 28, 2025

rishi-jat deleted the enhance/golangci-lint-configuration branch November 28, 2025 11:29

feat: enhance golangci-lint configuration for better code quality #367

feat: enhance golangci-lint configuration for better code quality #367

Uh oh!

Conversation

rishi-jat commented Oct 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elevran left a comment

Choose a reason for hiding this comment

Uh oh!

elevran commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rishi-jat commented Oct 24, 2025

Uh oh!

elevran commented Oct 26, 2025

Uh oh!

rishi-jat commented Oct 30, 2025

Uh oh!

elevran commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elevran Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rishi-jat Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

rishi-jat Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

elevran Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 26, 2025

Uh oh!

rishi-jat commented Nov 26, 2025

Uh oh!

elevran Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

elevran Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

rishi-jat commented Nov 28, 2025

Uh oh!

rishi-jat commented Nov 28, 2025

Uh oh!

elevran commented Nov 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rishi-jat commented Oct 4, 2025 •

edited

Loading

elevran commented Oct 24, 2025 •

edited

Loading

elevran commented Oct 31, 2025 •

edited

Loading

elevran Nov 4, 2025 •

edited

Loading