Skip to content

Conversation

adleong
Copy link
Member

@adleong adleong commented Jul 3, 2025

Replace the static FS generated by the vfs package with the go embed directive. These are the same in concept, except that the go embed directive is performed automatically by the go build system at build time instead of requiring a separate go generate step. Therefore this allows us to remove our use of go generate and avoids needing to manage generated sources.

@adleong adleong requested a review from a team as a code owner July 3, 2025 22:25
adleong added 5 commits July 3, 2025 22:26
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
@adleong adleong marked this pull request as draft July 8, 2025 23:53
adleong added 6 commits July 8, 2025 23:56
@adleong adleong changed the title chore(cli): check generated gogen files into git refactor(helm): Replace VFS with embed for Helm chart rendering Jul 11, 2025
@adleong adleong marked this pull request as ready for review July 11, 2025 02:05
Copy link
Member

@alpeb alpeb left a comment

Choose a reason for hiding this comment

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

This is beyond awesome! 💯

I confirmed it fixes #13873 :

# build the CLI in the main branch
$ rm target/cli/linux-amd64/linkerd
$ bin/build-cli-bin
$ sha256sum target/cli/linux-amd64/linkerd
6c70cbf6eabc8375bd085d11d4f01a2e2c6d7bbbdf54981c92db48ee4d2b6272  target/cli/linux-amd64/linkerd

# fetch charts again so the files timestamps change
$ rm -rf charts/
$ git checkout .
$ rm -rf target/cli/linux-amd64/linkerd
$ bin/build-cli-bin
# FILE CONTENTS HAVE CHANGED
$ sha256sum target/cli/linux-amd64/linkerd
2b95a8d78e2a52d71dd3f4f8d3b4bd5573b4927405cb278a3e27a01d2b9667f4  target/cli/linux-amd64/linkerd

# Let's do the same using the current branch
$ git checkout alex/template-prototype
$ rm target/cli/linux-amd64/linkerd
$ bin/build-cli-bin
$ sha256sum target/cli/linux-amd64/linkerd
582b5f47454e961a6c3c9736e8e1971c4463c94f9a361dd358b94b8be9112906  target/cli/linux-amd64/linkerd
# fetch charts again so the files timestamps change
$ rm -rf charts/
$ git checkout .
$ rm -rf target/cli/linux-amd64/linkerd
$ bin/build-cli-bin
# FILE CONTENTS HAVE ** NOT ** CHANGED!
$ sha256sum target/cli/linux-amd64/linkerd
582b5f47454e961a6c3c9736e8e1971c4463c94f9a361dd358b94b8be9112906  target/cli/linux-amd64/linkerd

Copy link
Member

@cratelyn cratelyn left a comment

Choose a reason for hiding this comment

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

this is excellent work, @adleong! 🙌

@adleong adleong merged commit 4125bd8 into main Jul 14, 2025
72 of 78 checks passed
@adleong adleong deleted the alex/template-prototype branch July 14, 2025 19:22
adleong added a commit that referenced this pull request Jul 15, 2025
adleong added a commit that referenced this pull request Jul 15, 2025
adleong added a commit that referenced this pull request Jul 24, 2025
adleong added a commit that referenced this pull request Jul 24, 2025
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.

4 participants