Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

goextensions/builder: glob match deps instead of string.Contains #1

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

ChenJesse
Copy link
Contributor

Before this commit, the way taskrunner works is:

  1. when you use builder.WrapWithGoBuild, it looks at the import graph and registers those as dependencies
  2. we set goBuildTask.Sources = append(goBuildTask.Sources, filepath.Join("/*.go"), "/config_override.json") which means that all Go files will trigger our invalidation logic
  3. when invalidation logic is triggered, we look for an import graph dependency that matches the Go file that was changed. One of the deps is pylon/infra/localdev, and unfortunately the playground main.go file path is pylon/infra/localdev/playground/main.go, which satisfies this invalidation logic

Instead, let's use globs to ensure that the matches only apply if the file is one level deep.

Before this commit, the way taskrunner works is:

1. when you use builder.WrapWithGoBuild, it looks at the import graph and registers those as dependencies
2. we set goBuildTask.Sources = append(goBuildTask.Sources, filepath.Join("**/*.go"), "**/config_override.json") which means that all Go files will trigger our invalidation logic
3. when invalidation logic is triggered, we look for an import graph dependency that matches the Go file that was changed. One of the deps is pylon/infra/localdev, and unfortunately the playground main.go file path is pylon/infra/localdev/playground/main.go, which satisfies this invalidation logic

Instead, let's use globs to ensure that the matches only apply if the file is one level deep.
@ChenJesse ChenJesse merged commit 7613c62 into main Aug 27, 2024
1 check passed
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.

1 participant