Rename doc + address comments

Ajay Kannan · Ajay Kannan · commit d4d4d5bcbcc0 · 2016-04-14T15:59:14.000-07:00
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -43,9 +43,9 @@ The feature must work fully on Java 7 and above.
 The feature must not add unnecessary dependencies (where "unnecessary" is of course subjective,
 but new dependencies should be discussed).
 
-Adding a New Client Library
----------------------------
-See [ADDING_NEW_CLIENTS](./ADDING_NEW_CLIENTS.md) for guidelines on how to add a new client library to `gcloud-java`.
+Adding Support for a New Service
+--------------------------------
+See [SUPPORTING_NEW_SERVICES](./SUPPORT_NEW_SERVICES.md) for guidelines on how to add a new client library to `gcloud-java`.
 
 Coding Style
 ------------
diff --git a/SUPPORTING_NEW_SERVICES.md b/SUPPORTING_NEW_SERVICES.md
@@ -1,4 +1,4 @@
-## How to Add a New Client Library
+## Supporting New Services
 
 ### Overview
 
@@ -18,7 +18,7 @@ This document outlines how to add a new client library to `gcloud-java`.  New cl
 Before starting work on the API layer, write a design document and provide sample API code, either in the design document or as a pull request tagged with the "don't merge" label.  As part of the design process, be sure to examine the Google Cloud service API and any implementations provided in other gcloud-* language libraries.  Solicit feedback from other contributors to the repository.  Client libraries should be low-level while minimizing boilerplate code needed by API users.  The library should also be flexible enough to be used by higher-level libraries.  For example, Objectify should be able to use `gcloud-java-datastore`.
 
 The library should contain:
-* A subclass of the [`ServiceOptions`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ServiceOptions.java) class. The `ServiceOptions` class contains important information for communicating with the Google Cloud service, such as the project ID, authentication, and retry handling.  Subclasses should add/override relevant methods as necessary.  Example: [`DatastoreOptions`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java)
+* A subclass of the [`ServiceOptions`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ServiceOptions.java) class. The `ServiceOptions` class contains important information for communicating with the Google Cloud service, such as the project ID, authentication, and retry handling.  Subclasses should add/override relevant methods as necessary.  Example: [`DatastoreOptions`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java).
 * An interface extending the [`Service`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/Service.java) interface (example: [`Datastore`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/Datastore.java)) and an implentation of that interface (example: [`DatastoreImpl`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java)).
 * An interface extending the [`ServiceFactory`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ServiceFactory.java) interface.  Example: [`DatastoreFactory`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreFactory.java)
 * A runtime exception class that extends [`BaseServiceException`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/BaseServiceException.java), which is used to wrap service-related exceptions.  Example: [`DatastoreException`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java)
@@ -29,7 +29,7 @@ In general, make classes immutable whenever possible, providing builders as nece
 `gcloud-java-core` provides functionality for code patterns used across `gcloud-java` libraries.  The following are some important core concepts:
 * Paging: Cloud services often expose page-based listing using page tokens.  The [`Page`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/Page.java) interface should be used for page-based listing.  A `Page` contains an iterator over results in that page, as well as methods to get the next page and all results in future pages.  `Page` requires a `NextPageFetcher` implementation (see the `NextPageFetcher` interface in [`PageImpl`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/PageImpl.java)).  This implementation should delegate constructing request options to the `nextRequestOptions` method in `PageImpl`.
 
-* Exception handling: we have the notion of retryable vs non-retryable exceptions and idempotent vs non-idempotent exceptions.  These are encapsulated by `BaseServiceException`.  Retryable error codes should be listed in the client's subclass of `BaseServiceException`, specifying whether exception is idempotent.  The subclass should also provide methods to translate the exceptions given by the underlying autogeneraged client library.  The [`ExceptionHandler`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ExceptionHandler.java) class intercepts and decides how to handle exceptions (based on the set of retryable error codes) when making RPC calls.
+* Exception handling: we have the notion of retryable vs non-retryable exceptions and idempotent vs non-idempotent exceptions.  These are encapsulated by `BaseServiceException`.  Retryable error codes should be listed in the client's subclass of `BaseServiceException`.  An exception should be considered retryable if it makes sense to retry (e.g. if there was a transient error, not a fundamentally invalid request) and if the exception is idempotent.  The subclass should also provide methods to translate the exceptions given by the underlying autogeneraged client library.  The [`ExceptionHandler`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-core/src/main/java/com/google/cloud/ExceptionHandler.java) class intercepts and retries RPC calls when retryable exceptions are encountered.
 
 * Batching: The [`BatchResult`](https://github.com/GoogleCloudPlatform/gcloud-java/blob/dns-alpha-batch/gcloud-java-core/src/main/java/com/google/cloud/BatchResult.java) class provides a simple way for users to combine RPC calls for performance enhancement.  Clients for services that provide batching should provide a batch class that contains methods similar to the methods in the `Service.java` subclass, but instead return a subclass of `BatchResult`.  Also provide an SPI-layer class to collect batch requests and submit the batch.
 
@@ -40,6 +40,8 @@ Notes/reminders:
 * While not all fields for model objects need to be exposed to the user, `gcloud-java` clients should get and set all relevant fields when making RPC calls to the Cloud service's API.  For example, since the `parent` field of Cloud Resource Manager Project objects is in alpha (at the time this is written) and not available to most users, `gcloud-java-resourcemanager` gets and sets the parent when interacting with the Cloud Resource Manager, but does not expose the parent to users.  As a result, the user won't accidentally unset the parent when updating a project.
 * Be aware of differences in "update" behavior and name update/replace methods accordingly in your API.  See #321 for context.
 * Do not expose third party libraries in the API.  This has been a design choice from the beginning of the project, and all existing clients adhere to this convention.
+* Member variable getters and builder setters should not use the prefixes "get" or "set".
+* Any service-generated IDs for model objects should be named `generatedId()`.
 
 #### SPI layer
 
@@ -55,7 +57,7 @@ Test helper classes should be located in the package `com.google.cloud.servicena
 There are three types of test helpers:
 * When a local emulator is already available, your test helper should launch that emulator and return service options to connect to that local emulator.  This enables both users and our own library to run unit tests easily.  An example of this type of helper is `LocalDatastoreHelper`.  Google Cloud Datastore provides a script that launches a local datastore, so `LocalDatastoreHelper` launches that script in a separate process when the user calls `start()`.
 
-* When there is no local emulator but the service is simple enough to write an emulator, you should do so.  The emulator should listen to a port for requests, process those requests, and send responses back, being as true to the actual service as possible.  Be sure to document differences between your emulator and the actual service.  Examples of this type of test helper are `LocalResourceManagerHelper` and `LocalDnsHelper`.
+* When there is no local emulator but the service is simple enough to write an emulator, you should do so.  The emulator should listen to a port for requests, process those requests, and send responses back, being as true to the actual service as possible.  Dependencies in this mock should be as lightweight as possible.  Be sure to document differences between your emulator and the actual service.  Examples of this type of test helper are `LocalResourceManagerHelper` and `LocalDnsHelper`.
 
 * When there is no local emulator and the service is too complex to write a solid emulator, the test helper should contain methods to get options and separate test data from other user data.  `RemoteStorageHelper` is an example of this type of test helper, since there is no local emulator for Google Cloud Storage (at the time that this is written) and because the Google Cloud Storage API is complex.  `RemoteStorageHelper` has methods to:
   * Get service options settings.
@@ -65,7 +67,7 @@ There are three types of test helpers:
 
 #### Tests
 
-API-level functionality should be well-covered by unit tests.  Coders and reviewers should examine test coverage to ensure that important code paths are not being left untested.  As of now, `gcloud-java` relies on integration tests to test the SPI layer.  Unit tests for the API layer should be located in the package `com.google.cloud.servicename`.  Integration tests should be placed in a separate package, `com.google.cloud.servicename.it`, which enables us to catch method access bugs.  Unit tests for the test helper should be placed in the package `com.google.cloud.servicename.testing`.  All unit tests run for pull requests, but integration tests are only run upon merging the pull request.  We only run integration tests upon merging pull requests to avoid decrypting and exposing credentials to anybody who can create a pull request from a fork.
+API-level functionality should be well-covered by unit tests.  Coders and reviewers should examine test coverage to ensure that important code paths are not being left untested.  As of now, `gcloud-java` relies on integration tests to test the SPI layer.  Unit tests for the API layer should be located in the package `com.google.cloud.servicename`.  Integration tests should be placed in a separate package, `com.google.cloud.servicename.it`, which enables us to catch method access bugs.  Unit tests for the test helper should be placed in the package `com.google.cloud.servicename.testing`.  All unit tests run for pull requests, but integration tests are only run upon merging the pull request.  We only run integration tests upon merging pull requests to avoid decrypting and exposing credentials to anybody who can create a pull request from a fork.  For this purpose, integration test file names must be prefixed with "IT" (e.g. `ITDnsTest`).
 
 Simple service-related tests should be added to [GoogleCloudPlatform/gcloud-java-examples](https://github.com/GoogleCloudPlatform/gcloud-java-examples/tree/master/test-apps).  To test releases and platform-specific bugs, it's valuable to deploy the apps in that repository on App Engine, Compute Engine, and from your own desktop.
 
@@ -93,7 +95,7 @@ New services should be created in a branch based on `master`.  The branch name s
 Create at least two milestones (stable and future) for your service and an issue tag with the service name.  Create issues for any to-do items and tag them appropriately.  This keeps an up-to-date short-term to-do list and also allows for longer term roadmaps.
 
 Be sure you've configured the base folder's `pom.xml` correctly.
-  * Add your module to the base directory's `pom.xml` file under the list of modules.
+  * Add your module to the base directory's `pom.xml` file under the list of modules (in alphabetical order).
   * Add your module to the javadoc packaging settings.  See PR #802 for an example.
   <!--- (TODO: uncomment when gcs-nio branch is merged into master) * Add your example to the assembler plugin.  PR #839 includes examples of using appassembler. --->
 
