Skip to content

fix: apply gofmt formatting to package doc comment in klog.go#435

Merged
k8s-ci-robot merged 1 commit intokubernetes:mainfrom
pierluigilenoci:fix/gofmt-klog-comment-indentation
Mar 19, 2026
Merged

fix: apply gofmt formatting to package doc comment in klog.go#435
k8s-ci-robot merged 1 commit intokubernetes:mainfrom
pierluigilenoci:fix/gofmt-klog-comment-indentation

Conversation

@pierluigilenoci
Copy link
Copy Markdown

@pierluigilenoci pierluigilenoci commented Mar 16, 2026

Summary

Fix mixed tab/space indentation in the package doc comment of klog.go (lines 59-105) introduced by #432, which caused the golangci-lint gofmt check to fail in CI.

To prevent this from happening again, a separate PR has been submitted to add lint as a required status check in Prow branch protection: kubernetes/test-infra#36671.

Root cause

See root cause analysis comment for details on why PR #432's pipeline didn't catch the formatting error.

Test plan

  • gofmt -d klog.go shows no diff
  • go build ./... compiles successfully

@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Mar 16, 2026
@k8s-ci-robot
Copy link
Copy Markdown

This issue is currently awaiting triage.

If klog contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 16, 2026
@pierluigilenoci
Copy link
Copy Markdown
Author

@pohly @harshanarayana please take a look

@pierluigilenoci pierluigilenoci requested a review from pohly March 16, 2026 14:46
@pierluigilenoci
Copy link
Copy Markdown
Author

pierluigilenoci commented Mar 17, 2026

Root cause analysis: why PR #432's pipeline didn't catch the formatting errors

1. Lint workflow never ran on PR #432 (primary cause)

PR #432 was opened from a fork (pierluigilenoci/klogkubernetes/klog). For fork PRs, GitHub Actions requires maintainer approval before executing workflows (security measure to prevent arbitrary code execution).

Evidence from the workflow run history:

The lint only executed after the merge, on the push event to main (2026-03-03), where it failed — but by then the PR was already merged.

2. Lint is not a required check for Prow/Tide

The kubernetes/klog repo uses Prow/Tide (k8s-ci-robot) for automated merging. Tide has its own list of required checks, configured in the kubernetes/test-infra repo. The GitHub Actions "Run lint" workflow is not listed as a required check for Tide. So even if the lint had run and failed, Tide would have still merged the PR as long as it had the lgtm + approved labels.

Timeline summary

Event Lint status
PR #432 opened (2025-12-15) Not executed (fork PR, requires maintainer approval)
PR #432 merged by Tide (2026-03-03) Merged with lgtm + approved only
Push to main (2026-03-03) Lint executed → FAILURE (gofmt)
PR #435 opened Fixes the formatting error

Suggestions to prevent this in the future

  1. Maintainers should approve workflow runs on fork PRs before merging
  2. Add lint as a required check in the Prow/Tide configuration (kubernetes/test-infra) — submitted as kubernetes/test-infra#36671
  3. Use local pre-commit hooks (as added in this PR) to catch issues before they even reach CI

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 17, 2026
@pierluigilenoci pierluigilenoci changed the title fix: gofmt formatting and add pre-commit hooks fix: apply gofmt formatting to package doc comment in klog.go Mar 17, 2026
@pierluigilenoci
Copy link
Copy Markdown
Author

@pohly the PR has been updated as requested:

  • Removed .pre-commit-config.yaml — the PR now only touches klog.go (gofmt fix)
  • Updated title and description accordingly
  • The infrastructure fix (adding lint as a required status check) is tracked separately in kubernetes/test-infra#36671, where all CI checks are passing

Could you re-review when you get a chance? Thanks!

Copy link
Copy Markdown

@pohly pohly left a comment

Choose a reason for hiding this comment

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

Confirmed locally that gofmt is happy with this results.

Please squash into a single commit.

@pohly
Copy link
Copy Markdown

pohly commented Mar 19, 2026

/lgtm
/approve

After kubernetes/test-infra#36671 got merged this should not merge the PR. Seems to have worked: the two lint jobs are shown as "Waiting for status to be reported" and "Required".

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 19, 2026
@pohly
Copy link
Copy Markdown

pohly commented Mar 19, 2026

/hold

For squashing.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 19, 2026
@pierluigilenoci pierluigilenoci force-pushed the fix/gofmt-klog-comment-indentation branch from f88ad3d to 713c737 Compare March 19, 2026 14:56
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 19, 2026
@pierluigilenoci
Copy link
Copy Markdown
Author

@pohly Squashed into a single commit. PR is ready for review.

Copy link
Copy Markdown

@pohly pohly left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold cancel

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Mar 19, 2026
@k8s-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pierluigilenoci, pohly

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit dbd6b2b into kubernetes:main Mar 19, 2026
20 checks passed
@pierluigilenoci pierluigilenoci deleted the fix/gofmt-klog-comment-indentation branch March 19, 2026 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants