Skip to content

Indent doc comments to avoid go doc false positives in generated apis/v1alpha1 types - #742

Open
knottnt wants to merge 2 commits into
aws-controllers-k8s:mainfrom
knottnt:fix/gofmt-applies-to-struct-fields
Open

knottnt wants to merge 2 commits into
aws-controllers-k8s:mainfrom
knottnt:fix/gofmt-applies-to-struct-fields

Conversation

@knottnt

@knottnt knottnt commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:
Regenerating a controller on Go 1.27+ produced spurious diffs in apis/*.go (and helm/crds) versus controllers generated on Go ≤1.26. The code-generator emits struct-field doc comments at column 0 with * list bullets. build-controller.sh then runs gofmt, and gofmt ≤1.26 reformatted those column-1 field comments (rewriting *→- and re-indenting), while gofmt 1.27+ leaves them alone (Go go/printer commit c1f0b9b, fixing golang/go#52605 — field comments aren't top-level doc comments). Result: output depended on which Go toolchain the contributor had installed.

Fix: Emit generated doc comments with a leading tab so they start past column 1. gofmt's doc-comment reformatting only triggers on column-1 comments, so now both Go 1.26 and 1.27 leave the bullets untouched, with no requirement for contributors to be on a specific Go version.

Note: As a result of this change the generated CRDs under helm/crds will still be altered to match the CRDs under config/crd/bases. This PR just makes that behavior consistent between Go toolchain versions.

Verified: Regenerated glue-controller under gofmt 1.26.5 and 1.27.1 — all generated apis/*.go files byte-identical across both; output idempotent under re-gofmt.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ack-prow
ack-prow Bot requested review from a-hilaly and jlbutler September 9, 2026 16:36
@ack-prow ack-prow Bot added the approved label Sep 9, 2026
@knottnt

knottnt commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

/test iam-controller-test

- Update docstring unit tests to validate tab placement
- Update GetDocumentation() to apply indentation to each comment line
@gustavodiaz7722

Copy link
Copy Markdown
Member

/LGTM

@ack-prow ack-prow Bot added the lgtm Indicates that a PR is ready to be merged. label Sep 9, 2026
@ack-prow

ack-prow Bot commented Sep 9, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gustavodiaz7722, knottnt

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

@knottnt

knottnt commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@ack-prow

ack-prow Bot commented Sep 9, 2026

Copy link
Copy Markdown

@knottnt: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
acm-controller-test e62c6e3 link true /test acm-controller-test

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@knottnt

knottnt commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

/override acm-controller-test

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

Labels

approved lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants