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

Add dagger to bazel-common. #207

Merged
merged 1 commit into from
Jan 17, 2025
Merged

Add dagger to bazel-common. #207

merged 1 commit into from
Jan 17, 2025

Conversation

copybara-service[bot]
Copy link

Add dagger to bazel-common.

PiperOrigin-RevId: 716712857
@copybara-service copybara-service bot merged commit 169bcbd into master Jan 17, 2025
@copybara-service copybara-service bot deleted the test_716378902 branch January 17, 2025 18:13
@ronshapiro
Copy link
Contributor

This is an old version of dagger. And dagger is already built with Bazel, does it need this?

@cpovirk
Copy link
Member

cpovirk commented Jan 21, 2025

Thanks, I should have noticed the older version and asked @mollyibot if that was chosen to match a version that was already used by J2CL(?).

As for Dagger's use of Bazel: Are you suggesting that we follow the instructions at https://github.com/google/dagger#bazel instead of setting it up this way? We had discussed that briefly, and I wasn't sure whether we'd still benefit from the #bazel approach when we're going to be declaring //third_party/java/dagger targets manually in bazel-common either way. It does seem that our current approach might affect the versions that Bazel chooses for other dependencies, ones that Dagger uses only from its annotation processor, and maybe that doesn't make a lot of sense. But maybe it will also be more amenable to automated upgrades (which Dependabot doesn't do for Bazel, admittedly, though we could set up Renovatebot), or maybe either style is equally easy to update?

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.

3 participants