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

@example Comments should refer to sample code #157

Closed
sduskis opened this issue Jun 1, 2018 · 4 comments
Closed

@example Comments should refer to sample code #157

sduskis opened this issue Jun 1, 2018 · 4 comments
Assignees
Labels
api: bigtable Issues related to the googleapis/nodejs-bigtable API. docs type: cleanup An internal cleanup or hygiene concern.

Comments

@sduskis
Copy link
Contributor

sduskis commented Jun 1, 2018

nodejs-bigtable has @example comments that include code. This is error prone, since it's not compiled and may get obsolete. Instead of inline code, all @examples should refer to a sample file and a tag within it.

nodejs-spanner has a great examples of this. Here's one example (link):

/**
 ...
 * @example <caption>include:samples/batch.js</caption>
 * region_tag:spanner_batch_client
*/
@sduskis sduskis added priority: p2 Moderately-important priority. Fix may not be included in next release. type: cleanup An internal cleanup or hygiene concern. docs labels Jun 1, 2018
@sduskis sduskis self-assigned this Jun 8, 2018
@vijay-qlogic
Copy link
Contributor

@sduskis Shall I start adding this @example comment? start with src/instance.js which has example code in sample/instances.js

@sduskis
Copy link
Contributor Author

sduskis commented Jun 20, 2018

@vijay-qlogic, please do.

@vijay-qlogic
Copy link
Contributor

@sduskis Currently src/instance.js has @example comments with multiple ways to call a method.
Like: using callback, promise (when callback is omitted)
And in sample/instances.js we have used async/await
Shall we update sample/instances.js to cover all these format? Or how we should go ahead?
your thoughts please...

@sduskis
Copy link
Contributor Author

sduskis commented Jun 21, 2018

The sample/instances.js is meant to show basic situations. If we need more options, then we probably need to create additional files somewhere. Do unit tests or system test work well enough for this type of thing?

@ghost ghost removed the priority: p2 Moderately-important priority. Fix may not be included in next release. label Oct 5, 2018
@google-cloud-label-sync google-cloud-label-sync bot added the api: bigtable Issues related to the googleapis/nodejs-bigtable API. label Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the googleapis/nodejs-bigtable API. docs type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

No branches or pull requests

2 participants