Adding JLBP-1 ("Minimize dependencies") #57
Conversation
library-best-practices/JLBP-1.md
Outdated
| and if you require all of the transitive dependencies. | ||
| - Avoid cyclic dependencies | ||
| - E.g., gRPC depends on OpenCensus which depends on gRPC. | ||
| - Maven can detect this. |
There was a problem hiding this comment.
It's a little stronger: Maven does not allow this. Build fails outright if there's a cyclic dependency.
There was a problem hiding this comment.
@saturnism Looking at grpc's dependencies, there is no cycle technically. It goes grpc-core -> opencensus-api -> grpc-context. Maybe you're talking about an inter-project cycle, which is actually not detectable by maven?
There was a problem hiding this comment.
Thinking about this more, I think it should be a separate item - it's more subtle than just minimizing dependencies. I have pulled it offline (see internal doc).
library-best-practices/JLBP-1.md
Outdated
| added by just adding a single dependency to your own library, | ||
| and if you require all of the transitive dependencies. | ||
| - Avoid cyclic dependencies | ||
| - E.g., gRPC depends on OpenCensus which depends on gRPC. |
There was a problem hiding this comment.
Google style eschews Latin. That is, spell out "For example" instead of "E.g."
|
Addressed feedback, PTAL |
|
cc @cpovirk |
| `mvn dependency:tree` (after running `mvn install -DskipTests` | ||
| to build the library) to see which transitive dependencies are | ||
| added by just adding a single dependency to your own library, | ||
| and if you require all of the transitive dependencies. |
There was a problem hiding this comment.
Is it worth expanding on "if you require all of the transitive dependencies"? Even if you exclude them in your project, any project that depends on yours will not see the "excludes" directives on transitive dependencies.
There was a problem hiding this comment.
I want to make sure I understand what you're saying.
Let's use the following example: say your library (C) depends on B which transitively depends on A, and in C you exclude A. It seems clear to me that if a user of C (say D) depends on C and also adds their own dependency on B, then they would pull in A (since they didn't add the exclusion of A). However, would D pull in A in the case where D only lists C as their dependency?
There was a problem hiding this comment.
Right. I believe that is true, because you don't want someone else to diasable a dependency that might be needed somewhere else. <exclude> is not persisted in a published artifacts pom.xml. But now I feel like I need to double check. Let me try it out.
There was a problem hiding this comment.
Sorry, I'm super wrong. I don't know why I thought this. Must have been some weird problem that I misattributed to this.
There was a problem hiding this comment.
Alrighty. I'm going to go ahead and merge as-is.
|
|
||
| - Use the smallest scope possible. For example, auto-value doesn't | ||
| need to use compile scope, and can instead use compile-only, | ||
| since it doesn't need to appear on the classpath of consumers. |
There was a problem hiding this comment.
I wouldn't recommend compile-only for auto-value and other annotations artifacts. It's caused problems for Guava users: See google/guava#2721 and various other bugs files against us.
For AutoValue specifically, I would recommend the pattern in its user guide, in which auto-value-annotations is a compile dependency and auto-value itself is an annotation processor (which apparently you can accomplish by making it provided, but that seems like not the most precise way).
I can send a PR if you'd like. Maybe you want it to include an example of when compile-only is the right thing? If so, maybe that's the servlet API case?
There was a problem hiding this comment.
I'm not familiar with auto-value details, but servlets are definitely not compile-only.
There was a problem hiding this comment.
Ah, I am going off Ray's comment in an internal thread. It sounds like I can expect to hear a response from you there later :)
There was a problem hiding this comment.
@cpovirk a PR would be much appreciated! If we don't truly have any examples where we should use compile-only, then we should probably remove the mention of it.
No description provided.