Move public Starlark API to defs.bzl #53
Conversation
artem-zinnatullin
changed the title
Move dependencies to WORKSPACE Fix buildifier subheaders in comments
artem-zinnatullin
force-pushed
the
az/defs
branch
from
15c988d to
c69c8c3
Compare
|
|
||
| # Runtime | ||
|
|
||
| ## Dependencies | ||
|
|
||
| load("//detekt:dependencies.bzl", "rules_detekt_dependencies") | ||
| ### Java |
There was a problem hiding this comment.
Huh. Yeah, it will work (I think) but rule users will be forced to declare all dependencies on their own. This is easy to reproduce with creating a completely separate workspace and attempting to use rules_detekt there. It works there of course and will work at the Lyft repo since all dependencies are declared there (I can only guess as a legal Lyft employee). However, at least rules_proto is far less common than rules_java.
In fact, the whole motivation behind #36 was to provide a way to use rules_detekt without declaring everything that is needed internally by the rule itself.
There was a problem hiding this comment.
Yeah, this is a bit lost on me as my understanding of included WORKSPACE is that it'll inflate their dependencies even if I don't have them in my WORKSPACE, I'll need to play with it a bit more…
There was a problem hiding this comment.
Sadly when you depend on an external repo, Bazel doesn't parse its WORKSPACE at all. So with those changes, like @arturdryomov mentioned, clients would have to replicate the set of dependencies.
The rules_kotlin repo is moving away from this problem (partly, that's not the intent afaik, just a side effect) by shipping a prebuilt worker (bazelbuild/rules_kotlin#271), maybe a similar solution could help here?
|
Closing then :) |
Attempt to put everything back in a single public api file to improve user experience, I'm still trying to remember exactly why did we do macros in separate files #36
I remember we were trying to break some circular dependencies and allow user to customize certain things, however so far this PR passes our CI setup here and works in Lyft project