Skip to content

Conversation

ivankatliarchuk
Copy link
Contributor

@ivankatliarchuk ivankatliarchuk commented Sep 30, 2025

What does it do ?

While was testing PR #5814 find out that generated code is missing update.

The change was introduced here #5659

Solution is from here https://www.alexedwards.net/blog/how-to-manage-tool-dependencies-in-go-1.24-plus#using-a-separate-modfile-for-tools

follow-up:

  • move vaccum tool under go.tools.mod
  • unit-tests for CRDs and generated files, so there is no need to execute validation manually

Motivation

Add unit-tests for CRD generators in follow-up code, similar to https://github.com/kubernetes-sigs/external-dns/tree/master/internal/gen/docs

More

  • Yes, this PR title follows Conventional Commits
  • Yes, I added unit tests
  • Yes, I updated end user documentation accordingly

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. scripts Issues or PRs related to internal scripts labels Sep 30, 2025
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 30, 2025
@coveralls
Copy link

coveralls commented Sep 30, 2025

Pull Request Test Coverage Report for Build 18130553619

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 22 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.02%) to 78.577%

Files with Coverage Reduction New Missed Lines %
endpoint.go 11 90.8%
zz_generated.deepcopy.go 11 0.0%
Totals Coverage Status
Change from base Build 18123647456: -0.02%
Covered Lines: 15816
Relevant Lines: 20128

💛 - Coveralls

@ivankatliarchuk ivankatliarchuk marked this pull request as draft September 30, 2025 10:21
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 30, 2025
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 30, 2025
@ivankatliarchuk ivankatliarchuk marked this pull request as ready for review September 30, 2025 12:08
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 30, 2025
tool (
github.com/google/yamlfmt/cmd/yamlfmt
github.com/mikefarah/yq/v4
sigs.k8s.io/controller-tools/cmd/controller-gen
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we have a version set here ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Like the v0.17.2 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this is just my understing

The selected lines in go.tool.mod list tools without versions because, in a Go toolchain file, the tool block specifies the import paths of command-line tools to be installed, not their versions. The actual versions are managed in the require block below, which ensures the correct versions are used when the tools are installed. This separation keeps the tool list clean and version management centralized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Example for vacuum

tool github.com/daveshanley/vacuum

and version is managed
Screenshot 2025-10-01 at 16 01 41

github.com/daveshanley/vacuum v0.17.8 // indirect

Copy link
Contributor Author

@ivankatliarchuk ivankatliarchuk Oct 1, 2025

Choose a reason for hiding this comment

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

Another tool example
Screenshot 2025-10-01 at 16 07 02

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll pin similar question mattermost/mattermost-plugin-starter-template#217 (comment). There are multiple links to read trhough pros/cons if you have spare time.

Copy link
Collaborator

Choose a reason for hiding this comment

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

To me also it's not a blocker, just weird.

Copy link
Collaborator

@mloiseleur mloiseleur left a comment

Choose a reason for hiding this comment

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

/lgtm
@AndrewCharlesHay @vflaux Any comments ?

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 4, 2025
@ivankatliarchuk
Copy link
Contributor Author

/approve

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 4, 2025
@ivankatliarchuk
Copy link
Contributor Author

/approve cancel

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from mloiseleur. For more information see the Code Review Process.

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

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 removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. scripts Issues or PRs related to internal scripts size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants