Skip to content
This repository has been archived by the owner on Apr 2, 2024. It is now read-only.

Fix the import of vfsgen #822

Merged
merged 1 commit into from
Oct 21, 2021
Merged

Fix the import of vfsgen #822

merged 1 commit into from
Oct 21, 2021

Conversation

cevian
Copy link
Contributor

@cevian cevian commented Oct 19, 2021

Right now vfsgen is imported in a file with +build ignore.
This cause go mod tidy to delete it from go.mod and therefore
breaks dependabot.

This patch simply adds the import to a built file.

Right now vfsgen is imported in a file with `+build ignore`.
This cause `go mod tidy` to delete it from go.mod and therefore
breaks dependabot.

This patch simply adds the import to a built file.
@cevian
Copy link
Contributor Author

cevian commented Oct 19, 2021

an alternative is to switch to go:embed. But that means we would require go 1.16. Thoughts?

Copy link
Member

@Harkishen-Singh Harkishen-Singh left a comment

Choose a reason for hiding this comment

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

Can we not just add vfsgen while runtime in CI? That will avoid adding to the mod file at all.

Copy link
Contributor

@paulfantom paulfantom left a comment

Choose a reason for hiding this comment

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

I like this approach as it is commonly used in golang and do not hide complexity in CI while allowing developers to use go mod tidy without any issues.

@paulfantom
Copy link
Contributor

Referencing shurcooL/vfsgen#83 just to have a GitHub link and revert this PR when upstream issue is solved.

@Harkishen-Singh
Copy link
Member

Harkishen-Singh commented Oct 20, 2021

If we include in file whose package is being used actively in Promscale, won't that include in the promscale binary and increase the size, even though its not utilized?

@cevian
Copy link
Contributor Author

cevian commented Oct 20, 2021

@Harkishen-Singh I don't believe the size difference is big enough to worry about

@Harkishen-Singh
Copy link
Member

Harkishen-Singh commented Oct 21, 2021

Yeah, i check and the size diff is negligible, so we can go ahead.

-rwxrwxr-x  1 harkishen harkishen  31M Oct 21 12:47 promscale_master
-rwxrwxr-x  1 harkishen harkishen  31M Oct 21 12:47 promscale_pr_822

@cevian
Copy link
Contributor Author

cevian commented Oct 21, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Oct 21, 2021

@bors bors bot merged commit 45cc66f into master Oct 21, 2021
@bors bors bot deleted the fix_vfsgen_import branch October 21, 2021 15:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants