Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: generate uniform service overviews #1475

Merged

Conversation

stephenplusplus
Copy link
Contributor

RE: #1471

@callmehiphop I wanted to get your thoughts before I move forward. The changes in this PR:

  • Removes custom constructor descriptions. They mostly were filler, slight variants of something like "This is how you interact with a {serviceName} object".
  • Removes constructor @example blocks. This was mostly used to show how to create a client-- that is now covered in the generated description.
  • Creates a template file that should be used by all constructors for their description. This file should be parsed with _.template(), using a common templating syntax. This is done because each description should be different, depending on the context the user is viewing the docs in: umbrellaMode or not.
  • Exports a function from scripts/docs.buildOverview.js that takes a class name, like bigquery, and returns the templated description. This returned description should be pre-pended to any existing description a constructor might have.

Constructor @example blocks are currently used to initialize variables that are used throughout the rest of the class's examples. Removing these blocks created a bit of a problem, so as a solution, in test/docs.js, it will prepend example code with a manual instantiation.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 9, 2016

The full set of options which can be passed to `<%= pkgJson.name %>` are
outlined in our
[Authentication guide](#/docs/<%= className %>/<%= pkgJson.version %>/guides/authentication).

This comment was marked as spam.

@callmehiphop
Copy link
Contributor

Where would the overview content be visible to the user? In the collapsable menu like today? Or before the constructor description?

@stephenplusplus
Copy link
Contributor Author

There is only one body of text that describes a service and its constructor. So the Getting Started and Overview sections are gone. All content would appear as part of the constructor's description.

@stephenplusplus stephenplusplus changed the title build-prep docs: generate uniform service overviews Aug 10, 2016
@stephenplusplus
Copy link
Contributor Author

This has been absorbed by #1479.

@stephenplusplus
Copy link
Contributor Author

@callmehiphop

@stephenplusplus
Copy link
Contributor Author

It looks like the overview changes from this PR weren't carried over into #1479. The docs site still show 3 different sections for each API;

  • Getting Started
  • Overview
  • Constructor description

There is only meant to be one now, under the label of the constructor, i.e.:

screen shot 2016-08-12 at 9 30 11 am

screen shot 2016-08-12 at 9 30 20 am

@stephenplusplus stephenplusplus force-pushed the spp--docs-build-prep branch 2 times, most recently from 4335de0 to df4f483 Compare August 12, 2016 14:54
@stephenplusplus
Copy link
Contributor Author

Updated this PR, PTAL.

I also stuck Datastore constants on the prototype... it was never very convenient accessing them:

var datastore = require('@google-cloud/datastore')({ ... });
datastore.int // undefined (until now)

Are there other constants I need to do this for?

Until we get this change in, I don't think we should officially "release" (publishing the release notes), since the docs right now are pretty confusing, e.g. the first example on the BigQuery module's page tells you to require('google-cloud').

@coveralls
Copy link

coveralls commented Aug 12, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 0e31d81 on stephenplusplus:spp--docs-build-prep into 66ac068 on GoogleCloudPlatform:master.

@@ -29,7 +29,7 @@
"docs": "node ./scripts/docs/packages.js",
"bundle": "node ./scripts/docs/bundle.js",
"lint": "jshint scripts/ packages/ system-test/ test/ && jscs packages/ system-test/ test/",
"test": "npm run docs && mocha test/docs.js packages/*/test/*.js",
"test": "npm run docs && npm run bundle && mocha test/docs.js packages/*/test/*.js",

This comment was marked as spam.

This comment was marked as spam.

@callmehiphop
Copy link
Contributor

@stephenplusplus LGTM, I think we should wait for an update to the docs site before merging this though so that our overview isn't collapsed.

@callmehiphop callmehiphop merged commit c835d33 into googleapis:master Aug 15, 2016
stephenplusplus added a commit to stephenplusplus/gcloud-node that referenced this pull request Aug 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants