Browse Source

Add a Javadoc section to CONTRIBUTING.md (#49836)

Introduce a set of contributor guidelines for when Javadoc should be written,
and when it shouldn't.
Rory Hunter 5 years ago
parent
commit
997b96aa48
1 changed files with 102 additions and 10 deletions
  1. 102 10
      CONTRIBUTING.md

+ 102 - 10
CONTRIBUTING.md

@@ -107,7 +107,7 @@ and `JAVA11_HOME`, and `JAVA12_HOME` available so that the tests can pass.
 `jrunscript` for jdk distributions.
 
 Elasticsearch uses the Gradle wrapper for its build. You can execute Gradle
-using the wrapper via the `gradlew` script on Unix systems or `gradlew.bat` 
+using the wrapper via the `gradlew` script on Unix systems or `gradlew.bat`
 script on Windows in the root of the repository. The examples below show the
 usage on Unix.
 
@@ -157,9 +157,9 @@ For IntelliJ, go to
 For Eclipse, go to `Preferences->Java->Installed JREs` and add `-ea` to
 `VM Arguments`.
 
-Some tests related to locale testing also require the flag 
+Some tests related to locale testing also require the flag
 `-Djava.locale.providers` to be set. Set the VM options/VM arguments for
-IntelliJ or Eclipse like describe above to use 
+IntelliJ or Eclipse like describe above to use
 `-Djava.locale.providers=SPI,COMPAT`.
 
 ### Java Language Formatting Guidelines
@@ -168,8 +168,8 @@ Java files in the Elasticsearch codebase are formatted with the Eclipse JDT
 formatter, using the [Spotless
 Gradle](https://github.com/diffplug/spotless/tree/master/plugin-gradle)
 plugin. This plugin is configured on a project-by-project basis, via
-`build.gradle` in the root of the repository. So long as at least one
-project is configured, the formatting check can be run explicitly with:
+`build.gradle` in the root of the repository. The formatting check can be
+run explicitly with:
 
     ./gradlew spotlessJavaCheck
 
@@ -205,7 +205,7 @@ Please follow these formatting guidelines:
   these are intended for use in documentation, so please make it clear what
   you have done, and only do this where the benefit clearly outweighs the
   decrease in consistency.
-* Note that JavaDoc and block comments i.e. `/* ... */` are not formatted,
+* Note that Javadoc and block comments i.e. `/* ... */` are not formatted,
   but line comments i.e `// ...` are.
 * There is an implicit rule that negative boolean expressions should use
   the form `foo == false` instead of `!foo` for better readability of the
@@ -231,15 +231,107 @@ from the command line.
 
 Sometimes Spotless will report a "misbehaving rule which can't make up its
 mind" and will recommend enabling the `paddedCell()` setting. If you
-enabled this settings and run the format check again,
+enabled this setting and run the format check again,
 Spotless will write files to
 `$PROJECT/build/spotless-diagnose-java/` to aid diagnosis. It writes
 different copies of the formatted files, so that you can see how they
 differ and infer what is the problem.
 
-The `paddedCell()` option is disabled for normal operation in order to
-detect any misbehaviour. You can enabled the option from the command line
-by running Gradle with `-Dspotless.paddedcell`.
+The `paddedCell()` option is disabled for normal operation so that any
+misbehaviour is detected, and not just suppressed. You can enabled the
+option from the command line by running Gradle with `-Dspotless.paddedcell`.
+
+### Javadoc
+
+Good Javadoc can help with navigating and understanding code. Elasticsearch
+has some guidelines around when to write Javadoc and when not to, but note
+that we don't want to be overly prescriptive. The intent of these guidelines
+is to be helpful, not to turn writing code into a chore.
+
+#### The short version
+
+   1. Always add Javadoc to new code.
+   2. Add Javadoc to existing code if you can.
+   3. Document the "why", not the "how", unless that's important to the
+      "why".
+   4. Don't document anything trivial or obvious (e.g. getters and
+      setters). In other words, the Javadoc should add some value.
+
+#### The long version
+
+   1. If you add a new Java package, please also add package-level
+      Javadoc that explains what the package is for. This can just be a
+      reference to a more foundational / parent package if appropriate. An
+      example would be a package hierarchy for a new feature or plugin -
+      the package docs could explain the purpose of the feature, any
+      caveats, and possibly some examples of configuration and usage.
+   2. New classes and interfaces must have class-level Javadoc that
+      describes their purpose. There are a lot of classes in the
+      Elasticsearch repository, and it's easier to navigate when you
+      can quickly find out what is the purpose of a class. This doesn't
+      apply to inner classes or interfaces, unless you expect them to be
+      explicitly used outside their parent class.
+   3. New public methods must have Javadoc, because they form part of the
+      contract between the class and its consumers. Similarly, new abstract
+      methods must have Javadoc because they are part of the contract
+      between a class and its subclasses. It's important that contributors
+      know why they need to implement a method, and the Javadoc should make
+      this clear. You don't need to document a method if it's overriding an
+      abstract method (either from an abstract superclass or an interface),
+      unless your implementation is doing something "unexpected" e.g. deviating
+      from the intent of the original method.
+   4. Following on from the above point, please add docs to existing public
+      methods if you are editing them, or to abstract methods if you can.
+   5. Non-public, non-abstract methods don't require Javadoc, but if you feel
+      that adding some would make it easier for other developers to
+      understand the code, or why it's written in a particular way, then please
+      do so.
+   6. Properties don't need to have Javadoc, but please add some if there's
+      something useful to say.
+   7. Javadoc should not go into low-level implementation details unless
+      this is critical to understanding the code e.g. documenting the
+      subtleties of the implementation of a private method. The point here
+      is that implementations will change over time, and the Javadoc is
+      less likely to become out-of-date if it only talks about the what is
+      the purpose of the code, not what it does.
+   8. Examples in Javadoc can be very useful, so feel free to add some if
+      you can reasonably do so i.e. if it takes a whole page of code to set
+      up an example, then Javadoc probably isn't the right place for it.
+      Longer or more elaborate examples are probably better suited
+      to the package docs.
+   9. Test methods are a good place to add Javadoc, because you can use it
+      to succinctly describe e.g. preconditions, actions and expectations
+      of the test, more easily that just using the test name alone. Please
+      consider documenting your tests in this way.
+   10. Sometimes you shouldn't add Javadoc:
+       1. Where it adds no value, for example where a method's
+          implementation is trivial such as with getters and setters, or a
+          method just delegates to another object.
+       2. However, you should still add Javadoc if there are caveats around
+          calling a method that are not immediately obvious from reading the
+          method's implementation in isolation.
+       3. You can omit Javadoc for simple classes, e.g. where they are a
+          simple container for some data. However, please consider whether a
+          reader might still benefit from some additional background, for
+          example about why the class exists at all.
+   11. Not all comments need to be Javadoc. Sometimes it will make more
+       sense to add comments in a method's body, for example due to important
+       implementation decisions or "gotchas". As a general guide, if some
+       information forms part of the contract between a method and its callers,
+       then it should go in the Javadoc, otherwise you might consider using
+       regular comments in the code. Remember as well that Elasticsearch
+       has extensive [user documentation](./docs), and it is not the role
+       of Javadoc to replace that.
+   12. Please still try to make class, method or variable names as
+       descriptive and concise as possible, as opposed to relying solely on
+       Javadoc to describe something.
+   13. Use `@link` and `@see` to add references, either to related
+       resources in the codebase or to relevant external resources.
+   14. If you need help writing Javadoc, just ask!
+
+Finally, use your judgement! Base your decisions on what will help other
+developers - including yourself, when you come back to some code
+3 months in the future, having forgotten how it works.
 
 ### License Headers