php-docs-samples icon indicating copy to clipboard operation
php-docs-samples copied to clipboard

Upgrade all samples to new sample format

Open bshaffer opened this issue 3 years ago • 2 comments

We have a new samples format that greatly reduces the complexity and overhead of maintaining samples by using reflection to invoke the sample method. We need to upgrade all samples to use this format.

Background We have two existing formats, the symfony/console format and the "functionless" format.

  1. The symfony/consoleformat: This format uses the symfony/console component to create an executable command file (e.g. vision.php) which executes the samples. This requires the overhead of adding every sample to the command file, and adds a layer of abstraction for the users.
  2. The "functionless" format: This format uses inline PHP to execute the sample. This has the advantage of making the sample files executable, but we lose the wrapping function, which is useful for consistency and documentation.

The New Format The new format uses a method in testing/sample_helpers.php to execute the function automatically from the CLI, and also to print usage instructions based on the PHP doc for the function. All that is required is these two lines at the bottom of each sample file.

Samples already upgraded

  • [x] Asset (#1311)
  • [x] Compute
  • [x] Firestore (#1363, #1381)
  • [x] Pubsub (#1356)
  • [x] IAP (#1378)
  • [x] IOT (#1379)
  • [x] Spanner (#1234)
  • [x] Auth (#1375)
  • [x] Storage (#1384)
  • [x] Bigtable (#1496)
  • [x] DLP (#1615)
  • [x] TextToSpeech (https://github.com/GoogleCloudPlatform/php-docs-samples/pull/1641)
  • [x] Video (9 samples) (in progress - https://github.com/GoogleCloudPlatform/php-docs-samples/pull/1639)
  • [x] SecretManager (15 samples) (https://github.com/GoogleCloudPlatform/php-docs-samples/pull/1647)
  • [x] Language (13 samples) (in progress - https://github.com/GoogleCloudPlatform/php-docs-samples/pull/1650)
  • [ ] vision (4 samples) (in progress - https://github.com/GoogleCloudPlatform/php-docs-samples/pull/1638)
  • [ ] monitoring (27 samples) (in progress - https://github.com/GoogleCloudPlatform/php-docs-samples/pull/1640)
  • [ ] logging (4 samples) (in progress - https://github.com/GoogleCloudPlatform/php-docs-samples/pull/1643)
  • [ ] Speech (14 samples) (in progress - https://github.com/GoogleCloudPlatform/php-docs-samples/pull/1642)
  • [ ] KMS (36 samples) (in progress - https://github.com/GoogleCloudPlatform/php-docs-samples/pull/1649)
  • [ ] datastore/tutorial (6 samples) (in progress - https://github.com/GoogleCloudPlatform/php-docs-samples/pull/1648)

Samples still using symfony/console format

  • [x] ~~appengine/flexible/tasks~~
  • [x] ~~appengine/flexible/wordpress~~
  • [x] ~~appengine/flexible/drupal8~~
  • [x] ~~appengine/flexible/symfony~~
  • [ ] endpoints/getting-started (1 sample app)
  • [ ] dialogflow (19 samples)

Samples using the "functionless" format

  • [ ] BigQuery (28 samples)
  • [ ] Error Reporting (1 samples)
  • [ ] Translate (21 samples)

bshaffer avatar Jun 16 '21 17:06 bshaffer

All that is required is these two lines at the bottom of each sample file.

This seems a bit "un-natural" for us to do.

As a thought, having this functionality in a trait seems more "Object-Oriented" style? It would also be much easier for the users(and more visible) to remove the trait in their original code. Something like the following comes to mind:

class ListClusterSample{
  use Google\Cloud\Samples\SampleHelperTrait;
  
  // samples
}

(new ListClusterSample())->executeSample();

saranshdhingra avatar Jun 21 '21 13:06 saranshdhingra

@saranshdhingra The primary purpose is to illustrate how to perform whatever function the sample is there to illustrate. Surrounding the sample in an object and including a trait will make the sample itself confusing and take away from this primary purpose.

The direction we have chosen allows the sample to remain entirely unchanged except for two lines at the bottom of the file, which are excluded from the region tags.

bshaffer avatar Jun 21 '21 14:06 bshaffer