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

Initial open-sourcing of add_dep tool #1269

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

cushon
Copy link
Contributor

@cushon cushon commented May 13, 2024

See bazelbuild/bazel#4990

This is a lightly modified fork of the internal tool, with updates to e.g. use bazel instead of blaze, and use the standard flags package, etc. I confirmed that it works on examples like bazelbuild/bazel#4990.

One open question here is test coverage. The internal implementation has a suite of e2e integration tests that can be run manually. They set up a tiny bazel workspace, do a build that produces an add_dep error, and confirm that running the provided command fixes the build. I could look at externalizing those. It would be nicer to have something that could run on CI.

Copy link

@sluongng sluongng left a comment

Choose a reason for hiding this comment

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

you probably want to move each of these files to a separate directory to follow the convention of 1 dir == 1 package. Or to rename all the packages to add_dep.
This way the repo can still be built using go install.

@@ -0,0 +1,68 @@
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test")

Choose a reason for hiding this comment

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

Suggested change
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test")
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")

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.

2 participants