JLBP: Declare all dependencies #1767
Conversation
| Code should not call methods, reference fields, or instantiate classes from _indirect_ dependencies. These are dependencies of the declared dependencies. Projects that rely on indirect dependencies have an annoying habit of breaking in unexpected ways when direct dependencies are upgraded. | ||
|
|
||
| For example, your project might declare a dependency on the | ||
| Google HTTP Java Client which |
There was a problem hiding this comment.
| Google HTTP Java Client which | |
| Google HTTP Java Client, which |
There was a problem hiding this comment.
I think this is a restrictive clause, but it's arguable.
There was a problem hiding this comment.
If it were restrictive, that would mean that there are several Google HTTP Java Clients, only one of which depends on Apache HTTP Components. Is that the case?
There was a problem hiding this comment.
That's one way of identifying restrictive subordinate clauses, but a little more generally, "A restrictive adjective clause, on the other hand, is essential to a sentence and should not be set off by commas" https://www.thoughtco.com/restrictive-and-nonrestrictive-adjective-clauses-1689689
I think this clause is indeed essential to the sentence.
A better reference than that will have to wait until I get back to the office to retrieve my Chicago Manual of Style. :-)
There was a problem hiding this comment.
The content of the clause is important to the meaning of the paragraph, but not essential to the sentence.
"For example, your project might declare a dependency on the Google HTTP Java Client."
That doesn't change the reader's understanding of what you mean by "the Google HTTP Java Client", although it leaves out an important detail about why you're talking about it.
Compare to your sentence above: "Projects that rely on indirect dependencies have an annoying habit of breaking in unexpected ways when direct dependencies are upgraded." If you got rid of the restrictive clause and changed it to "Projects have an annoying habit of breaking in unexpected ways when direct dependencies are upgraded", the reader wouldn't know which projects you were talking about.
Anyway, it's your document.
There was a problem hiding this comment.
I think it needs a comma for the reason David explained:
If it were restrictive, that would mean that there are several Google HTTP Java Clients, only one of which depends on Apache HTTP Components
There was a problem hiding this comment.
The point is "essential to a sentence," which clause this is. It is not whether it is essential to the meaning of the modified noun or noun phrase.
docs/JLBP-0022.md
Outdated
| explicitly declaring a dependency on Apache HTTP Components. However, you should | ||
| add the dependency anyway. This way if a future version of the | ||
| Google HTTP Java Client no longer depends on Apache HTTP Components, your code will | ||
| still compile and work. Strict dependencies also help static analysis tools better understand a project. |
There was a problem hiding this comment.
I think you could start a new paragraph with "Strict dependencies...." It's starting a new idea.
|
|
||
| # Declare all dependencies | ||
|
|
||
| If your code references a class—for example, by invoking a method in that class—declare a dependency that includes that class in your pom.xml, build.gradle, |
There was a problem hiding this comment.
The varying line lengths make this hard to review. Can you run mdformat?
There was a problem hiding this comment.
-bash: mdformat: command not found
docs/JLBP-0022.md
Outdated
| still compile and work. Strict dependencies also help static analysis tools better understand a project. | ||
|
|
||
| This doesn't only happen when a project itself is upgraded. It can also | ||
| happen when dependency mediation selects a different version of a library's dependency |
There was a problem hiding this comment.
Maybe give examples of when dependency mediation might change versions other than when you change your declared dependency versions.
There was a problem hiding this comment.
I'm hesitant to have a single sentence paragraph.
|
|
||
| # Declare all dependencies | ||
|
|
||
| If your code references a class—for example, by invoking a method in that class—declare a dependency that includes that class in your pom.xml, build.gradle, |
There was a problem hiding this comment.
Is there a formatting practice for referring to files? Should this be pom.xml or pom.xml?
There was a problem hiding this comment.
I don't think we've chosen one yet. Might be worth checking general google style and general cleanup.
docs/JLBP-0022.md
Outdated
| permalink: /JLBP-22 | ||
| --- | ||
|
|
||
| # Declare all dependencies |
There was a problem hiding this comment.
Should this refer explicitly to direct dependencies somehow?
|
"https://jlbp.dev/JLBP-1 Minimize Dependencies" discusses the number of dependencies but in a different direction. I think it's better to combine the two into one such as "Declare optimal number of dependencies". What do you think? |
|
JLBP-1 is not the same. That's about whether to depend on something. This is about how to declare the dependencies you have. |
| Code should not call methods, reference fields, or instantiate classes from _indirect_ dependencies. These are dependencies of the declared dependencies. Projects that rely on indirect dependencies have an annoying habit of breaking in unexpected ways when direct dependencies are upgraded. | ||
|
|
||
| For example, your project might declare a dependency on the | ||
| Google HTTP Java Client which |
There was a problem hiding this comment.
If it were restrictive, that would mean that there are several Google HTTP Java Clients, only one of which depends on Apache HTTP Components. Is that the case?
| Code should not call methods, reference fields, or instantiate classes from _indirect_ dependencies. These are dependencies of the declared dependencies. Projects that rely on indirect dependencies have an annoying habit of breaking in unexpected ways when direct dependencies are upgraded. | ||
|
|
||
| For example, your project might declare a dependency on the | ||
| Google HTTP Java Client which |
There was a problem hiding this comment.
I think it needs a comma for the reason David explained:
If it were restrictive, that would mean that there are several Google HTTP Java Clients, only one of which depends on Apache HTTP Components
| explicitly declaring a dependency on Apache HTTP Components. However, you should | ||
| add the dependency anyway. This way if a future version of the | ||
| Google HTTP Java Client no longer depends on Apache HTTP Components, your code | ||
| still compiles. Strict dependencies also help static analysis tools better understand a project. |
There was a problem hiding this comment.
Would you add names of these static analysis tools you have in mind?
There was a problem hiding this comment.
If I'm asked from somebody about this static analysis tools, I cannot answer it. Would you help me in this regard?
| explicitly declaring a dependency on Apache HTTP Components. However, you should | ||
| add the dependency anyway. This way if a future version of the | ||
| Google HTTP Java Client no longer depends on Apache HTTP Components, your code | ||
| still compiles. Strict dependencies also help static analysis tools better understand a project. |
There was a problem hiding this comment.
"Strict deps" appears in a previous paragraph, but 'Strict dependencies" does not. Would you pick one?
There was a problem hiding this comment.
bazel does call it "strict deps" bit otherwise we try not to abbreviate
There was a problem hiding this comment.
Bazel call it in that way, but this document does not name this practice.
@suztomo @netdpb fixes #1542