bazeltools / bazel-deps

Generate bazel dependencies for maven artifacts
MIT License
249 stars 121 forks source link

document why we transitively build all the targets #98

Open johnynek opened 6 years ago

johnynek commented 6 years ago
oscar [15:33] 
the reason I did it was simple: in a big repo all kinds of dependencies will be needed

[15:33] 
so, just pruning the graph back to a minimal set of targets is a pain

[15:33] 
if you just let bazel see all the granularity you don't have to worry

[15:33] 
so, we use cats-core and circe, which uses cats-core, no problem
ittaiz commented 6 years ago

You mean build all targets of external dependencies? On Tue, 28 Nov 2017 at 3:39 P. Oscar Boykin notifications@github.com wrote:

oscar [15:33] the reason I did it was simple: in a big repo all kinds of dependencies will be needed

[15:33] so, just pruning the graph back to a minimal set of targets is a pain

[15:33] if you just let bazel see all the granularity you don't have to worry

[15:33] so, we use cats-core and circe, which uses cats-core, no problem

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/johnynek/bazel-deps/issues/98, or mute the thread https://github.com/notifications/unsubscribe-auth/ABUIF9pE3cffVUHlj1ZcDdTBtSnCPRGEks5s62RngaJpZM4Qsjxl .

johnynek commented 6 years ago

Sorry this was unclear. Why do we make BUILD rules for all the transitive jars? That is the question. The alternative is to just make a few targets you think you need.

ittaiz commented 6 years ago

Oh I see, thanks for the clarification On Tue, 28 Nov 2017 at 7:39 P. Oscar Boykin notifications@github.com wrote:

Sorry this was unclear. Why do we make BUILD rules for all the transitive jars? That is the question. The alternative is to just make a few targets you think you need.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/johnynek/bazel-deps/issues/98#issuecomment-347419440, or mute the thread https://github.com/notifications/unsubscribe-auth/ABUIFxidr7i0CoCFt321cT8lrk30ZcXoks5s65x9gaJpZM4Qsjxl .

kamalmarhubi commented 6 years ago

Related to this, I think that artifacts not explicitly listed in the dependencies file should have visibility = ["//path/to/generated/build/files:__subpackages__"] to force all dependencies to be explicitly mentioned. Let me know if I should open that as another issue.

johnynek commented 6 years ago

It would be nice to use visibility. I don't know much about it personally. I would love to make transitive dependencies not explicitly declared in the yaml non-public.

johnynek commented 6 years ago

(so an issue would be great to track, and a PR even better! :)

kamalmarhubi commented 6 years ago

Filed as https://github.com/johnynek/bazel-deps/issues/99

Will look into opening a PR.